Skip to content

Commit

Permalink
Release underlying resources when JS instance is GC'ed on Android try…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
janicduplessis authored and grabbou committed Oct 11, 2019
1 parent 3a96f4c commit 766f470
Show file tree
Hide file tree
Showing 10 changed files with 214 additions and 12 deletions.
1 change: 1 addition & 0 deletions ReactAndroid/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ def getNdkBuildFullPath() {
task buildReactNdkLib(dependsOn: [prepareJSC, prepareHermes, prepareBoost, prepareDoubleConversion, prepareFolly, prepareGlog], type: Exec) {
inputs.dir("$projectDir/../ReactCommon")
inputs.dir("src/main/jni")
inputs.dir("src/main/java/com/facebook/react/modules/blob")
outputs.dir("$buildDir/react-ndk/all")
commandLine(getNdkBuildFullPath(),
"NDK_DEBUG=" + (nativeBuildType.equalsIgnoreCase("debug") ? "1" : "0"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ rn_android_library(
],
deps = [
react_native_dep("libraries/fbcore/src/main/java/com/facebook/common/logging:logging"),
react_native_dep("libraries/soloader/java/com/facebook/soloader:soloader"),
react_native_dep("third-party/java/infer-annotations:infer-annotations"),
react_native_dep("third-party/java/jsr-305:jsr-305"),
react_native_dep("third-party/java/okhttp:okhttp3"),
Expand All @@ -24,6 +25,7 @@ rn_android_library(
react_native_target("java/com/facebook/react/bridge:bridge"),
react_native_target("java/com/facebook/react/common:common"),
react_native_target("java/com/facebook/react/module/annotations:annotations"),
react_native_target("java/com/facebook/react/modules/blob/jni:jni"),
react_native_target("java/com/facebook/react/modules/network:network"),
react_native_target("java/com/facebook/react/modules/websocket:websocket"),
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.facebook.react.modules.blob;

import com.facebook.react.bridge.JavaScriptContextHolder;
import com.facebook.react.bridge.ReactContext;
import com.facebook.soloader.SoLoader;

/* package */ class BlobCollector {
static {
SoLoader.loadLibrary("reactnativeblob");
}

static void install(final ReactContext reactContext, final BlobModule blobModule) {
reactContext.runOnJSQueueThread(
new Runnable() {
@Override
public void run() {
JavaScriptContextHolder jsContext = reactContext.getJavaScriptContextHolder();
// When debugging in chrome the JS context is not available.
if (jsContext.get() != 0) {
nativeInstall(blobModule, jsContext.get());
}
}
});
}

private static native void nativeInstall(Object blobModule, long jsContext);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import android.provider.MediaStore;
import android.webkit.MimeTypeMap;
import androidx.annotation.Nullable;
import com.facebook.proguard.annotations.DoNotStrip;
import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
Expand Down Expand Up @@ -143,6 +144,11 @@ public BlobModule(ReactApplicationContext reactContext) {
super(reactContext);
}

@Override
public void initialize() {
BlobCollector.install(getReactApplicationContext(), this);
}

@Override
public String getName() {
return NAME;
Expand Down Expand Up @@ -170,11 +176,16 @@ public String store(byte[] data) {
}

public void store(byte[] data, String blobId) {
mBlobs.put(blobId, data);
synchronized (mBlobs) {
mBlobs.put(blobId, data);
}
}

@DoNotStrip
public void remove(String blobId) {
mBlobs.remove(blobId);
synchronized (mBlobs) {
mBlobs.remove(blobId);
}
}

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

public @Nullable byte[] resolve(String blobId, int offset, int size) {
byte[] data = mBlobs.get(blobId);
if (data == null) {
return null;
}
if (size == -1) {
size = data.length - offset;
}
if (offset > 0 || size != data.length) {
data = Arrays.copyOfRange(data, offset, offset + size);
synchronized (mBlobs) {
byte[] data = mBlobs.get(blobId);
if (data == null) {
return null;
}
if (size == -1) {
size = data.length - offset;
}
if (offset > 0 || size != data.length) {
data = Arrays.copyOfRange(data, offset, offset + size);
}
return data;
}
return data;
}

public @Nullable byte[] resolve(ReadableMap blob) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# 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.

LOCAL_PATH := $(call my-dir)

include $(CLEAR_VARS)

LOCAL_MODULE := reactnativeblob

LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp)

LOCAL_C_INCLUDES := $(LOCAL_PATH)

LOCAL_CFLAGS += -fvisibility=hidden -fexceptions -frtti

LOCAL_STATIC_LIBRARIES := libjsi libjsireact
LOCAL_SHARED_LIBRARIES := libfolly_json libfb libreactnativejni

include $(BUILD_SHARED_LIBRARY)
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
load("//tools/build_defs/oss:rn_defs.bzl", "ANDROID", "FBJNI_TARGET", "react_native_target", "react_native_xplat_dep", "rn_xplat_cxx_library")

rn_xplat_cxx_library(
name = "jni",
srcs = glob(["*.cpp"]),
headers = glob(["*.h"]),
header_namespace = "",
compiler_flags = [
"-fexceptions",
"-frtti",
"-std=c++14",
"-Wall",
],
platforms = ANDROID,
soname = "libreactnativeblob.$(ext)",
visibility = ["PUBLIC"],
deps = [
react_native_target("jni/react/jni:jni"),
FBJNI_TARGET,
react_native_xplat_dep("jsi:jsi"),
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2004-present Facebook. All Rights Reserved.
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

#include "BlobCollector.h"

#include <fb/fbjni.h>
#include <memory>
#include <mutex>

using namespace facebook;

namespace facebook {
namespace react {

static constexpr auto kBlobModuleJavaDescriptor =
"com/facebook/react/modules/blob/BlobModule";

BlobCollector::BlobCollector(
jni::global_ref<jobject> blobModule,
const std::string &blobId)
: blobModule_(blobModule), blobId_(blobId) {}

BlobCollector::~BlobCollector() {
jni::ThreadScope::WithClassLoader([&] {
static auto removeMethod = jni::findClassStatic(kBlobModuleJavaDescriptor)
->getMethod<void(jstring)>("remove");
removeMethod(blobModule_, jni::make_jstring(blobId_).get());
});
}

void BlobCollector::nativeInstall(
jni::alias_ref<jhybridobject> jThis,
jni::alias_ref<jobject> blobModule,
jlong jsContextNativePointer) {
auto &runtime = *((jsi::Runtime *) jsContextNativePointer);
auto blobModuleRef = jni::make_global(blobModule);
runtime.global().setProperty(
runtime,
"__blobCollectorProvider",
jsi::Function::createFromHostFunction(
runtime,
jsi::PropNameID::forAscii(runtime, "__blobCollectorProvider"),
1,
[blobModuleRef](
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<BlobCollector>(blobModuleRef, blobId);
return jsi::Object::createFromHostObject(rt, blobCollector);
}));
}

void BlobCollector::registerNatives() {
registerHybrid(
{makeNativeMethod("nativeInstall", BlobCollector::nativeInstall)});
}

} // namespace react
} // namespace facebook
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2004-present Facebook. All Rights Reserved.
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

#pragma once

#include <fb/fbjni.h>
#include <jsi/jsi.h>

namespace facebook {
namespace react {

class BlobCollector : public jni::HybridClass<BlobCollector>,
public jsi::HostObject {
public:
BlobCollector(
jni::global_ref<jobject> blobManager,
const std::string &blobId);
~BlobCollector();

static constexpr auto kJavaDescriptor =
"Lcom/facebook/react/modules/blob/BlobCollector;";

static void nativeInstall(
jni::alias_ref<jhybridobject> jThis,
jni::alias_ref<jobject> blobModule,
jlong jsContextNativePointer);

static void registerNatives();

private:
friend HybridBase;

jni::global_ref<jobject> blobModule_;
const std::string blobId_;
};

} // namespace react
} // namespace facebook
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2004-present Facebook. All Rights Reserved.

// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

#include <fb/fbjni.h>

#include "BlobCollector.h"

JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
return facebook::jni::initialize(
vm, [] { facebook::react::BlobCollector::registerNatives(); });
}
1 change: 1 addition & 0 deletions ReactAndroid/src/main/jni/react/jni/Android.mk
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,4 @@ include $(REACT_SRC_DIR)/turbomodule/core/jni/Android.mk
include $(REACT_SRC_DIR)/jscexecutor/Android.mk
include $(REACT_SRC_DIR)/../hermes/reactexecutor/Android.mk
include $(REACT_SRC_DIR)/../hermes/instrumentation/Android.mk
include $(REACT_SRC_DIR)/modules/blob/jni/Android.mk

0 comments on commit 766f470

Please sign in to comment.