Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Blob] Release underlying resources when JS instance in GC'ed on iOS #24405

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions Libraries/Blob/BlobManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -31,6 +31,21 @@ 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 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.
*/
Expand Down Expand Up @@ -94,7 +109,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,
});
}

/**
Expand Down
3 changes: 3 additions & 0 deletions Libraries/Blob/BlobTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@

'use strict';

export opaque type BlobCollector = {};

export type BlobData = {
blobId: string,
offset: number,
size: number,
name?: string,
type?: string,
lastModified?: number,
__collector?: ?BlobCollector,
};

export type BlobOptions = {
Expand Down
13 changes: 13 additions & 0 deletions Libraries/Blob/RCTBlob.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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 = "<group>"; };
19461729225F085900E4E008 /* RCTBlobCollector.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTBlobCollector.mm; sourceTree = "<group>"; };
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 = "<group>"; };
AD9A43C21DFC7126008DC588 /* RCTBlobManager.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTBlobManager.mm; sourceTree = "<group>"; };
Expand All @@ -61,6 +67,8 @@
358F4ECE1D1E81A9004DF814 = {
isa = PBXGroup;
children = (
19461728225F085900E4E008 /* RCTBlobCollector.h */,
19461729225F085900E4E008 /* RCTBlobCollector.mm */,
ADDFBA6A1F33455F0064C998 /* RCTFileReaderModule.h */,
ADDFBA6B1F33455F0064C998 /* RCTFileReaderModule.m */,
AD9A43C11DFC7126008DC588 /* RCTBlobManager.h */,
Expand Down Expand Up @@ -89,6 +97,7 @@
buildActionMask = 2147483647;
files = (
AD0871161E215EC9007D136D /* RCTBlobManager.h in Headers */,
1946172A225F085900E4E008 /* RCTBlobCollector.h in Headers */,
ADDFBA6C1F33455F0064C998 /* RCTFileReaderModule.h in Headers */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand All @@ -98,6 +107,7 @@
buildActionMask = 2147483647;
files = (
19BA88FF1F84392900741C5A /* RCTFileReaderModule.h in Headers */,
1946172B225F085900E4E008 /* RCTBlobCollector.h in Headers */,
AD0871181E215ED1007D136D /* RCTBlobManager.h in Headers */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down Expand Up @@ -162,6 +172,7 @@
developmentRegion = English;
hasScannedForEncodings = 0;
knownRegions = (
English,
en,
);
mainGroup = 358F4ECE1D1E81A9004DF814;
Expand All @@ -180,6 +191,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
1946172C225F085900E4E008 /* RCTBlobCollector.mm in Sources */,
ADDFBA6D1F33455F0064C998 /* RCTFileReaderModule.m in Sources */,
AD9A43C31DFC7126008DC588 /* RCTBlobManager.mm in Sources */,
);
Expand All @@ -189,6 +201,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
1946172D225F085900E4E008 /* RCTBlobCollector.mm in Sources */,
19BA89011F84393D00741C5A /* RCTFileReaderModule.m in Sources */,
ADD01A711E09404A00F6D226 /* RCTBlobManager.mm in Sources */,
);
Expand Down
30 changes: 30 additions & 0 deletions Libraries/Blob/RCTBlobCollector.h
Original file line number Diff line number Diff line change
@@ -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 <jsi/jsi.h>

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
52 changes: 52 additions & 0 deletions Libraries/Blob/RCTBlobCollector.mm
Original file line number Diff line number Diff line change
@@ -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 <React/RCTBridge+Private.h>
#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];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this dance of using blobId as string, then asking nativemodule to clean it up, it's probably better long term to send down direct pointer to a blob (as jsi::Value) to JS, which can get cleaned up automatically by the JS runtime. See Fabric's UIManager method/impl as an example.

});
}

void RCTBlobCollector::install(RCTBlobManager *blobManager) {
__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,
"__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<RCTBlobCollector>(blobManager, blobId);
return jsi::Object::createFromHostObject(rt, blobCollector);
}
)
);
} queue:RCTJSThread];
}

} // namespace react
} // namespace facebook
4 changes: 4 additions & 0 deletions Libraries/Blob/RCTBlobManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#import <React/RCTNetworking.h>
#import <React/RCTUtils.h>
#import <React/RCTWebSocketModule.h>
#import "RCTBlobCollector.h"

static NSString *const kBlobURIScheme = @"blob";

Expand All @@ -33,13 +34,16 @@ @implementation RCTBlobManager
RCT_EXPORT_MODULE(BlobModule)

@synthesize bridge = _bridge;
@synthesize methodQueue = _methodQueue;

- (void)setBridge:(RCTBridge *)bridge
{
_bridge = bridge;

std::lock_guard<std::mutex> lock(_blobsMutex);
_blobs = [NSMutableDictionary new];

facebook::react::RCTBlobCollector::install(self);
}

+ (BOOL)requiresMainQueueSetup
Expand Down