Skip to content

Commit 9b2374b

Browse files
janicduplessisfacebook-github-bot
authored andcommitted
Release underlying resources when JS instance is GC'ed on Android try 2 (#26155)
Summary: Reland #24767 The commit had to be reverted because it caused a crash when using remote debugging in chrome. This is normal since jsi is not available in that environment. The crash was caused by `jsContext.get()` being 0, then being dereferenced later in c++. We can simply skip initializing the blob collector in this case. This also includes the fix from #25720 to fix a crash when using hermes. ## Changelog [Android] [Fixed] - Release underlying resources when JS instance is GC'ed on Android Pull Request resolved: #26155 Test Plan: Test using RN tester with jsc and hermes Test remote debugging Reviewed By: mdvacca, fred2028 Differential Revision: D17072644 Pulled By: makovkastar fbshipit-source-id: 079d1d43501e854297fbbe586ba229920c892584
1 parent 811401b commit 9b2374b

File tree

10 files changed

+214
-12
lines changed

10 files changed

+214
-12
lines changed

ReactAndroid/build.gradle

+1
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ def buildReactNdkLib = tasks.register("buildReactNdkLib", Exec) {
257257
dependsOn(prepareJSC, prepareHermes, prepareBoost, prepareDoubleConversion, prepareFolly, prepareGlog)
258258
inputs.dir("$projectDir/../ReactCommon")
259259
inputs.dir("src/main/jni")
260+
inputs.dir("src/main/java/com/facebook/react/modules/blob")
260261
outputs.dir("$buildDir/react-ndk/all")
261262
commandLine(getNdkBuildFullPath(),
262263
"NDK_DEBUG=" + (nativeBuildType.equalsIgnoreCase("debug") ? "1" : "0"),

ReactAndroid/src/main/java/com/facebook/react/modules/blob/BUCK

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ rn_android_library(
1616
],
1717
deps = [
1818
react_native_dep("libraries/fbcore/src/main/java/com/facebook/common/logging:logging"),
19+
react_native_dep("libraries/soloader/java/com/facebook/soloader:soloader"),
1920
react_native_dep("third-party/java/infer-annotations:infer-annotations"),
2021
react_native_dep("third-party/java/jsr-305:jsr-305"),
2122
react_native_dep("third-party/java/okhttp:okhttp3"),
@@ -24,6 +25,7 @@ rn_android_library(
2425
react_native_target("java/com/facebook/react/bridge:bridge"),
2526
react_native_target("java/com/facebook/react/common:common"),
2627
react_native_target("java/com/facebook/react/module/annotations:annotations"),
28+
react_native_target("java/com/facebook/react/modules/blob/jni:jni"),
2729
react_native_target("java/com/facebook/react/modules/network:network"),
2830
react_native_target("java/com/facebook/react/modules/websocket:websocket"),
2931
],
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package com.facebook.react.modules.blob;
2+
3+
import com.facebook.react.bridge.JavaScriptContextHolder;
4+
import com.facebook.react.bridge.ReactContext;
5+
import com.facebook.soloader.SoLoader;
6+
7+
/* package */ class BlobCollector {
8+
static {
9+
SoLoader.loadLibrary("reactnativeblob");
10+
}
11+
12+
static void install(final ReactContext reactContext, final BlobModule blobModule) {
13+
reactContext.runOnJSQueueThread(
14+
new Runnable() {
15+
@Override
16+
public void run() {
17+
JavaScriptContextHolder jsContext = reactContext.getJavaScriptContextHolder();
18+
// When debugging in chrome the JS context is not available.
19+
if (jsContext.get() != 0) {
20+
nativeInstall(blobModule, jsContext.get());
21+
}
22+
}
23+
});
24+
}
25+
26+
private static native void nativeInstall(Object blobModule, long jsContext);
27+
}

ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java

+25-12
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import android.provider.MediaStore;
1313
import android.webkit.MimeTypeMap;
1414
import androidx.annotation.Nullable;
15+
import com.facebook.proguard.annotations.DoNotStrip;
1516
import com.facebook.react.bridge.Arguments;
1617
import com.facebook.react.bridge.ReactApplicationContext;
1718
import com.facebook.react.bridge.ReactContextBaseJavaModule;
@@ -143,6 +144,11 @@ public BlobModule(ReactApplicationContext reactContext) {
143144
super(reactContext);
144145
}
145146

147+
@Override
148+
public void initialize() {
149+
BlobCollector.install(getReactApplicationContext(), this);
150+
}
151+
146152
@Override
147153
public String getName() {
148154
return NAME;
@@ -170,11 +176,16 @@ public String store(byte[] data) {
170176
}
171177

172178
public void store(byte[] data, String blobId) {
173-
mBlobs.put(blobId, data);
179+
synchronized (mBlobs) {
180+
mBlobs.put(blobId, data);
181+
}
174182
}
175183

184+
@DoNotStrip
176185
public void remove(String blobId) {
177-
mBlobs.remove(blobId);
186+
synchronized (mBlobs) {
187+
mBlobs.remove(blobId);
188+
}
178189
}
179190

180191
public @Nullable byte[] resolve(Uri uri) {
@@ -193,17 +204,19 @@ public void remove(String blobId) {
193204
}
194205

195206
public @Nullable byte[] resolve(String blobId, int offset, int size) {
196-
byte[] data = mBlobs.get(blobId);
197-
if (data == null) {
198-
return null;
199-
}
200-
if (size == -1) {
201-
size = data.length - offset;
202-
}
203-
if (offset > 0 || size != data.length) {
204-
data = Arrays.copyOfRange(data, offset, offset + size);
207+
synchronized (mBlobs) {
208+
byte[] data = mBlobs.get(blobId);
209+
if (data == null) {
210+
return null;
211+
}
212+
if (size == -1) {
213+
size = data.length - offset;
214+
}
215+
if (offset > 0 || size != data.length) {
216+
data = Arrays.copyOfRange(data, offset, offset + size);
217+
}
218+
return data;
205219
}
206-
return data;
207220
}
208221

209222
public @Nullable byte[] resolve(ReadableMap blob) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Copyright (c) Facebook, Inc. and its affiliates.
2+
#
3+
# This source code is licensed under the MIT license found in the
4+
# LICENSE file in the root directory of this source tree.
5+
6+
LOCAL_PATH := $(call my-dir)
7+
8+
include $(CLEAR_VARS)
9+
10+
LOCAL_MODULE := reactnativeblob
11+
12+
LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp)
13+
14+
LOCAL_C_INCLUDES := $(LOCAL_PATH)
15+
16+
LOCAL_CFLAGS += -fvisibility=hidden -fexceptions -frtti
17+
18+
LOCAL_STATIC_LIBRARIES := libjsi libjsireact
19+
LOCAL_SHARED_LIBRARIES := libfolly_json libfb libreactnativejni
20+
21+
include $(BUILD_SHARED_LIBRARY)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
load("//tools/build_defs/oss:rn_defs.bzl", "ANDROID", "FBJNI_TARGET", "react_native_target", "react_native_xplat_dep", "rn_xplat_cxx_library")
2+
3+
rn_xplat_cxx_library(
4+
name = "jni",
5+
srcs = glob(["*.cpp"]),
6+
headers = glob(["*.h"]),
7+
header_namespace = "",
8+
compiler_flags = [
9+
"-fexceptions",
10+
"-frtti",
11+
"-std=c++14",
12+
"-Wall",
13+
],
14+
platforms = ANDROID,
15+
soname = "libreactnativeblob.$(ext)",
16+
visibility = ["PUBLIC"],
17+
deps = [
18+
react_native_target("jni/react/jni:jni"),
19+
FBJNI_TARGET,
20+
react_native_xplat_dep("jsi:jsi"),
21+
],
22+
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright 2004-present Facebook. All Rights Reserved.
2+
// This source code is licensed under the MIT license found in the
3+
// LICENSE file in the root directory of this source tree.
4+
5+
#include "BlobCollector.h"
6+
7+
#include <fb/fbjni.h>
8+
#include <memory>
9+
#include <mutex>
10+
11+
using namespace facebook;
12+
13+
namespace facebook {
14+
namespace react {
15+
16+
static constexpr auto kBlobModuleJavaDescriptor =
17+
"com/facebook/react/modules/blob/BlobModule";
18+
19+
BlobCollector::BlobCollector(
20+
jni::global_ref<jobject> blobModule,
21+
const std::string &blobId)
22+
: blobModule_(blobModule), blobId_(blobId) {}
23+
24+
BlobCollector::~BlobCollector() {
25+
jni::ThreadScope::WithClassLoader([&] {
26+
static auto removeMethod = jni::findClassStatic(kBlobModuleJavaDescriptor)
27+
->getMethod<void(jstring)>("remove");
28+
removeMethod(blobModule_, jni::make_jstring(blobId_).get());
29+
});
30+
}
31+
32+
void BlobCollector::nativeInstall(
33+
jni::alias_ref<jhybridobject> jThis,
34+
jni::alias_ref<jobject> blobModule,
35+
jlong jsContextNativePointer) {
36+
auto &runtime = *((jsi::Runtime *) jsContextNativePointer);
37+
auto blobModuleRef = jni::make_global(blobModule);
38+
runtime.global().setProperty(
39+
runtime,
40+
"__blobCollectorProvider",
41+
jsi::Function::createFromHostFunction(
42+
runtime,
43+
jsi::PropNameID::forAscii(runtime, "__blobCollectorProvider"),
44+
1,
45+
[blobModuleRef](
46+
jsi::Runtime &rt,
47+
const jsi::Value &thisVal,
48+
const jsi::Value *args,
49+
size_t count) {
50+
auto blobId = args[0].asString(rt).utf8(rt);
51+
auto blobCollector =
52+
std::make_shared<BlobCollector>(blobModuleRef, blobId);
53+
return jsi::Object::createFromHostObject(rt, blobCollector);
54+
}));
55+
}
56+
57+
void BlobCollector::registerNatives() {
58+
registerHybrid(
59+
{makeNativeMethod("nativeInstall", BlobCollector::nativeInstall)});
60+
}
61+
62+
} // namespace react
63+
} // namespace facebook
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2004-present Facebook. All Rights Reserved.
2+
// This source code is licensed under the MIT license found in the
3+
// LICENSE file in the root directory of this source tree.
4+
5+
#pragma once
6+
7+
#include <fb/fbjni.h>
8+
#include <jsi/jsi.h>
9+
10+
namespace facebook {
11+
namespace react {
12+
13+
class BlobCollector : public jni::HybridClass<BlobCollector>,
14+
public jsi::HostObject {
15+
public:
16+
BlobCollector(
17+
jni::global_ref<jobject> blobManager,
18+
const std::string &blobId);
19+
~BlobCollector();
20+
21+
static constexpr auto kJavaDescriptor =
22+
"Lcom/facebook/react/modules/blob/BlobCollector;";
23+
24+
static void nativeInstall(
25+
jni::alias_ref<jhybridobject> jThis,
26+
jni::alias_ref<jobject> blobModule,
27+
jlong jsContextNativePointer);
28+
29+
static void registerNatives();
30+
31+
private:
32+
friend HybridBase;
33+
34+
jni::global_ref<jobject> blobModule_;
35+
const std::string blobId_;
36+
};
37+
38+
} // namespace react
39+
} // namespace facebook
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2004-present Facebook. All Rights Reserved.
2+
3+
// This source code is licensed under the MIT license found in the
4+
// LICENSE file in the root directory of this source tree.
5+
6+
#include <fb/fbjni.h>
7+
8+
#include "BlobCollector.h"
9+
10+
JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
11+
return facebook::jni::initialize(
12+
vm, [] { facebook::react::BlobCollector::registerNatives(); });
13+
}

ReactAndroid/src/main/jni/react/jni/Android.mk

+1
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,4 @@ include $(REACT_SRC_DIR)/turbomodule/core/jni/Android.mk
7979
include $(REACT_SRC_DIR)/jscexecutor/Android.mk
8080
include $(REACT_SRC_DIR)/../hermes/reactexecutor/Android.mk
8181
include $(REACT_SRC_DIR)/../hermes/instrumentation/Android.mk
82+
include $(REACT_SRC_DIR)/modules/blob/jni/Android.mk

0 commit comments

Comments
 (0)