Skip to content

Commit

Permalink
Release underlying resources when JS instance in GC'ed (#24745)
Browse files Browse the repository at this point in the history
Summary:
Our Blob implementation was very problematic because it didn't release its underlying resource when the JS instance was dealocated. The main issue is that the fetch polyfill uses blobs by default if the module is available, which causes large memory leaks.

This fixes it by using the new jsi infra to attach a `jsi::HostObject` (`BlobCollector`)  to `Blob` instances. This way when the `Blob` is collected, the `BlobCollector` also gets collected. Using the `jsi::HostObject` dtor we can schedule the cleanup of native resources. This is definitely not the ideal solution but otherwise it would require rewriting the whole module using TurboModules + jsi.

Fixes #23801, #20352, #21092

[General] [Fixed] - [Blob] Release underlying resources when JS instance in GC'ed
Pull Request resolved: #24745

Reviewed By: fkgozali

Differential Revision: D15248848

Pulled By: hramos

fbshipit-source-id: 1da835cc935dfbf4e7bb6fbf2aea29bfdc9bd6fa
  • Loading branch information
janicduplessis authored and facebook-github-bot committed May 8, 2019
1 parent f2618fd commit 9ef5107
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 2 deletions.
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('../BatchedBridge/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
15 changes: 15 additions & 0 deletions Libraries/Blob/RCTBlob.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@
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 */; };
19BA89011F84393D00741C5A /* RCTFileReaderModule.m in Sources */ = {isa = PBXBuildFile; fileRef = ADDFBA6B1F33455F0064C998 /* RCTFileReaderModule.m */; };
19D9CA2622820DA40021BD26 /* RCTBlobCollector.h in Copy Headers */ = {isa = PBXBuildFile; fileRef = 19461728225F085900E4E008 /* RCTBlobCollector.h */; };
AD0871131E215B28007D136D /* RCTBlobManager.h in Copy Headers */ = {isa = PBXBuildFile; fileRef = AD9A43C11DFC7126008DC588 /* RCTBlobManager.h */; };
AD0871161E215EC9007D136D /* RCTBlobManager.h in Headers */ = {isa = PBXBuildFile; fileRef = AD9A43C11DFC7126008DC588 /* RCTBlobManager.h */; };
AD0871181E215ED1007D136D /* RCTBlobManager.h in Headers */ = {isa = PBXBuildFile; fileRef = AD9A43C11DFC7126008DC588 /* RCTBlobManager.h */; };
Expand All @@ -28,6 +33,7 @@
dstPath = include/RCTBlob;
dstSubfolderSpec = 16;
files = (
19D9CA2622820DA40021BD26 /* RCTBlobCollector.h in Copy Headers */,
19BA88FE1F84391700741C5A /* RCTFileReaderModule.h in Copy Headers */,
AD08711A1E2162C8007D136D /* RCTBlobManager.h in Copy Headers */,
);
Expand All @@ -49,6 +55,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 +69,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 +99,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 +109,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 +174,7 @@
developmentRegion = English;
hasScannedForEncodings = 0;
knownRegions = (
English,
en,
);
mainGroup = 358F4ECE1D1E81A9004DF814;
Expand All @@ -180,6 +193,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 +203,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];
});
}

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

0 comments on commit 9ef5107

Please sign in to comment.