From cc62559c8d93707188575f7fbb4b85c2068ca2c7 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Thu, 11 Apr 2019 01:36:52 -0400 Subject: [PATCH 1/3] [Blob] Release underlying resources when JS instance in GC'ed --- Libraries/Blob/BlobManager.js | 29 ++++++++++- Libraries/Blob/BlobTypes.js | 3 ++ .../Blob/RCTBlob.xcodeproj/project.pbxproj | 13 +++++ Libraries/Blob/RCTBlobCollector.h | 30 +++++++++++ Libraries/Blob/RCTBlobCollector.mm | 52 +++++++++++++++++++ Libraries/Blob/RCTBlobManager.mm | 4 ++ 6 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 Libraries/Blob/RCTBlobCollector.h create mode 100644 Libraries/Blob/RCTBlobCollector.mm diff --git a/Libraries/Blob/BlobManager.js b/Libraries/Blob/BlobManager.js index 460980bb5b21ef..f1b331121ea4cc 100644 --- a/Libraries/Blob/BlobManager.js +++ b/Libraries/Blob/BlobManager.js @@ -14,7 +14,7 @@ const Blob = require('Blob'); const BlobRegistry = require('BlobRegistry'); const {BlobModule} = require('NativeModules'); -import type {BlobData, BlobOptions} from 'BlobTypes'; +import type {BlobData, BlobOptions, BlobCollector} from 'BlobTypes'; /*eslint-disable no-bitwise */ /*eslint-disable eqeqeq */ @@ -31,6 +31,20 @@ function uuidv4(): string { }); } +// Blob collector is a jsi::HostObject that is used by native to know +// when the a Blob instance is deallocated. This allows to free the +// underlying native resources. This is a hack to workaround the fact +// that the current bridge infra doesn't allow to track js objects +// deallocation. Ideally this would use TurboModules and the whole Blob +// object should be a jsi::HostObject. +function createBlobCollector(blobId: string): BlobCollector | null { + if (global.__blobCollectorProvider == null) { + return null; + } else { + return global.__blobCollectorProvider(blobId); + } +} + /** * Module to manage blobs. Wrapper around the native blob module. */ @@ -94,7 +108,18 @@ class BlobManager { */ static createFromOptions(options: BlobData): Blob { BlobRegistry.register(options.blobId); - return Object.assign(Object.create(Blob.prototype), {data: options}); + return Object.assign(Object.create(Blob.prototype), { + data: + // Reuse the collector instance when creating from an existing blob. + // This will make sure that the underlying resource is only deallocated + // when all blobs that refer to it are deallocated. + options.__collector == null + ? { + ...options, + __collector: createBlobCollector(options.blobId), + } + : options, + }); } /** diff --git a/Libraries/Blob/BlobTypes.js b/Libraries/Blob/BlobTypes.js index de105d2a6904d3..cbd49b90160a1d 100644 --- a/Libraries/Blob/BlobTypes.js +++ b/Libraries/Blob/BlobTypes.js @@ -10,6 +10,8 @@ 'use strict'; +export opaque type BlobCollector = {}; + export type BlobData = { blobId: string, offset: number, @@ -17,6 +19,7 @@ export type BlobData = { name?: string, type?: string, lastModified?: number, + __collector?: ?BlobCollector, }; export type BlobOptions = { diff --git a/Libraries/Blob/RCTBlob.xcodeproj/project.pbxproj b/Libraries/Blob/RCTBlob.xcodeproj/project.pbxproj index 8bb32fec7b57fa..03ca9f9a1cfe9a 100755 --- a/Libraries/Blob/RCTBlob.xcodeproj/project.pbxproj +++ b/Libraries/Blob/RCTBlob.xcodeproj/project.pbxproj @@ -7,6 +7,10 @@ objects = { /* Begin PBXBuildFile section */ + 1946172A225F085900E4E008 /* RCTBlobCollector.h in Headers */ = {isa = PBXBuildFile; fileRef = 19461728225F085900E4E008 /* RCTBlobCollector.h */; }; + 1946172B225F085900E4E008 /* RCTBlobCollector.h in Headers */ = {isa = PBXBuildFile; fileRef = 19461728225F085900E4E008 /* RCTBlobCollector.h */; }; + 1946172C225F085900E4E008 /* RCTBlobCollector.mm in Sources */ = {isa = PBXBuildFile; fileRef = 19461729225F085900E4E008 /* RCTBlobCollector.mm */; }; + 1946172D225F085900E4E008 /* RCTBlobCollector.mm in Sources */ = {isa = PBXBuildFile; fileRef = 19461729225F085900E4E008 /* RCTBlobCollector.mm */; }; 19BA88FE1F84391700741C5A /* RCTFileReaderModule.h in Copy Headers */ = {isa = PBXBuildFile; fileRef = ADDFBA6A1F33455F0064C998 /* RCTFileReaderModule.h */; }; 19BA88FF1F84392900741C5A /* RCTFileReaderModule.h in Headers */ = {isa = PBXBuildFile; fileRef = ADDFBA6A1F33455F0064C998 /* RCTFileReaderModule.h */; }; 19BA89001F84392F00741C5A /* RCTFileReaderModule.h in Copy Headers */ = {isa = PBXBuildFile; fileRef = ADDFBA6A1F33455F0064C998 /* RCTFileReaderModule.h */; }; @@ -49,6 +53,8 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + 19461728225F085900E4E008 /* RCTBlobCollector.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTBlobCollector.h; sourceTree = ""; }; + 19461729225F085900E4E008 /* RCTBlobCollector.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTBlobCollector.mm; sourceTree = ""; }; 358F4ED71D1E81A9004DF814 /* libRCTBlob.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libRCTBlob.a; sourceTree = BUILT_PRODUCTS_DIR; }; AD9A43C11DFC7126008DC588 /* RCTBlobManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; lineEnding = 0; path = RCTBlobManager.h; sourceTree = ""; }; AD9A43C21DFC7126008DC588 /* RCTBlobManager.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTBlobManager.mm; sourceTree = ""; }; @@ -61,6 +67,8 @@ 358F4ECE1D1E81A9004DF814 = { isa = PBXGroup; children = ( + 19461728225F085900E4E008 /* RCTBlobCollector.h */, + 19461729225F085900E4E008 /* RCTBlobCollector.mm */, ADDFBA6A1F33455F0064C998 /* RCTFileReaderModule.h */, ADDFBA6B1F33455F0064C998 /* RCTFileReaderModule.m */, AD9A43C11DFC7126008DC588 /* RCTBlobManager.h */, @@ -89,6 +97,7 @@ buildActionMask = 2147483647; files = ( AD0871161E215EC9007D136D /* RCTBlobManager.h in Headers */, + 1946172A225F085900E4E008 /* RCTBlobCollector.h in Headers */, ADDFBA6C1F33455F0064C998 /* RCTFileReaderModule.h in Headers */, ); runOnlyForDeploymentPostprocessing = 0; @@ -98,6 +107,7 @@ buildActionMask = 2147483647; files = ( 19BA88FF1F84392900741C5A /* RCTFileReaderModule.h in Headers */, + 1946172B225F085900E4E008 /* RCTBlobCollector.h in Headers */, AD0871181E215ED1007D136D /* RCTBlobManager.h in Headers */, ); runOnlyForDeploymentPostprocessing = 0; @@ -162,6 +172,7 @@ developmentRegion = English; hasScannedForEncodings = 0; knownRegions = ( + English, en, ); mainGroup = 358F4ECE1D1E81A9004DF814; @@ -180,6 +191,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 1946172C225F085900E4E008 /* RCTBlobCollector.mm in Sources */, ADDFBA6D1F33455F0064C998 /* RCTFileReaderModule.m in Sources */, AD9A43C31DFC7126008DC588 /* RCTBlobManager.mm in Sources */, ); @@ -189,6 +201,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 1946172D225F085900E4E008 /* RCTBlobCollector.mm in Sources */, 19BA89011F84393D00741C5A /* RCTFileReaderModule.m in Sources */, ADD01A711E09404A00F6D226 /* RCTBlobManager.mm in Sources */, ); diff --git a/Libraries/Blob/RCTBlobCollector.h b/Libraries/Blob/RCTBlobCollector.h new file mode 100644 index 00000000000000..ccb9b7a891940f --- /dev/null +++ b/Libraries/Blob/RCTBlobCollector.h @@ -0,0 +1,30 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import + +using namespace facebook; + +@class RCTBlobManager; + +namespace facebook { +namespace react { + +class JSI_EXPORT RCTBlobCollector : public jsi::HostObject { + public: + RCTBlobCollector(RCTBlobManager *blobManager, const std::string &blobId); + ~RCTBlobCollector(); + + static void install(RCTBlobManager *blobManager); + + private: + const std::string blobId_; + RCTBlobManager *blobManager_; +}; + +} // namespace react +} // namespace facebook diff --git a/Libraries/Blob/RCTBlobCollector.mm b/Libraries/Blob/RCTBlobCollector.mm new file mode 100644 index 00000000000000..4d8c897367d239 --- /dev/null +++ b/Libraries/Blob/RCTBlobCollector.mm @@ -0,0 +1,52 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "RCTBlobCollector.h" + +#import +#import "RCTBlobManager.h" + +namespace facebook { +namespace react { + +RCTBlobCollector::RCTBlobCollector(RCTBlobManager *blobManager, const std::string &blobId) +: blobId_(blobId), blobManager_(blobManager) {} + +RCTBlobCollector::~RCTBlobCollector() { + RCTBlobManager *blobManager = blobManager_; + NSString *blobId = [NSString stringWithUTF8String:blobId_.c_str()]; + dispatch_async([blobManager_ methodQueue], ^{ + [blobManager remove:blobId]; + }); +} + +void RCTBlobCollector::install(RCTBlobManager *blobManager) { + RCTCxxBridge *cxxBridge = (RCTCxxBridge *)blobManager.bridge; + if (cxxBridge.runtime == nullptr) { + return; + } + [cxxBridge dispatchBlock:^{ + jsi::Runtime &runtime = *(jsi::Runtime *)cxxBridge.runtime; + runtime.global().setProperty( + runtime, + "__blobCollectorProvider", + jsi::Function::createFromHostFunction( + runtime, + jsi::PropNameID::forAscii(runtime, "__blobCollectorProvider"), + 1, + [blobManager](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) { + auto blobId = args[0].asString(rt).utf8(rt); + auto blobCollector = std::make_shared(blobManager, blobId); + return jsi::Object::createFromHostObject(rt, blobCollector); + } + ) + ); + } queue:RCTJSThread]; +} + +} // namespace react +} // namespace facebook diff --git a/Libraries/Blob/RCTBlobManager.mm b/Libraries/Blob/RCTBlobManager.mm index 597d9b23d4edbb..9e8faeb7b149ca 100755 --- a/Libraries/Blob/RCTBlobManager.mm +++ b/Libraries/Blob/RCTBlobManager.mm @@ -13,6 +13,7 @@ #import #import #import +#import "RCTBlobCollector.h" static NSString *const kBlobURIScheme = @"blob"; @@ -33,6 +34,7 @@ @implementation RCTBlobManager RCT_EXPORT_MODULE(BlobModule) @synthesize bridge = _bridge; +@synthesize methodQueue = _methodQueue; - (void)setBridge:(RCTBridge *)bridge { @@ -40,6 +42,8 @@ - (void)setBridge:(RCTBridge *)bridge std::lock_guard lock(_blobsMutex); _blobs = [NSMutableDictionary new]; + + facebook::react::RCTBlobCollector::install(self); } + (BOOL)requiresMainQueueSetup From 92a9f028c6b66aa83f7269df1a5726994bb92214 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Mon, 29 Apr 2019 17:34:18 -0400 Subject: [PATCH 2/3] Update BlobManager.js --- Libraries/Blob/BlobManager.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Libraries/Blob/BlobManager.js b/Libraries/Blob/BlobManager.js index f1b331121ea4cc..a59fa2af8f33ee 100644 --- a/Libraries/Blob/BlobManager.js +++ b/Libraries/Blob/BlobManager.js @@ -31,12 +31,13 @@ function uuidv4(): string { }); } +// **Temporary workaround** +// TODO(#24654): Use turbomodules for the Blob module. // Blob collector is a jsi::HostObject that is used by native to know // when the a Blob instance is deallocated. This allows to free the // underlying native resources. This is a hack to workaround the fact // that the current bridge infra doesn't allow to track js objects -// deallocation. Ideally this would use TurboModules and the whole Blob -// object should be a jsi::HostObject. +// deallocation. Ideally the whole Blob object should be a jsi::HostObject. function createBlobCollector(blobId: string): BlobCollector | null { if (global.__blobCollectorProvider == null) { return null; From f87571c034e7e201365243958c2b536acf4bc5cf Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Thu, 2 May 2019 14:12:53 -0400 Subject: [PATCH 3/3] Tweak global injection --- Libraries/Blob/RCTBlobCollector.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Libraries/Blob/RCTBlobCollector.mm b/Libraries/Blob/RCTBlobCollector.mm index 4d8c897367d239..284b52b5a4e8dd 100644 --- a/Libraries/Blob/RCTBlobCollector.mm +++ b/Libraries/Blob/RCTBlobCollector.mm @@ -25,11 +25,11 @@ } void RCTBlobCollector::install(RCTBlobManager *blobManager) { - RCTCxxBridge *cxxBridge = (RCTCxxBridge *)blobManager.bridge; - if (cxxBridge.runtime == nullptr) { - return; - } + __weak RCTCxxBridge *cxxBridge = (RCTCxxBridge *)blobManager.bridge; [cxxBridge dispatchBlock:^{ + if (!cxxBridge || cxxBridge.runtime == nullptr) { + return; + } jsi::Runtime &runtime = *(jsi::Runtime *)cxxBridge.runtime; runtime.global().setProperty( runtime,