From dbf9d084e4ae2817b39d2a1afddb6b646fd29d69 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Fri, 31 May 2019 03:51:56 -0700 Subject: [PATCH] Release underlying resources when JS instance is GC'ed on Android (#24767) Summary: Android followup for #24745. This adds a jsi object that removes blobs when it is gc'ed. We don't have many modules with native code on Android so I've added the native code directly in the blob package as a separate .so. I used a similar structure as the turbomodule package. ## Changelog [Android] [Fixed] - [Blob] Release underlying resources when JS instance is GC'ed on Android Pull Request resolved: https://github.com/facebook/react-native/pull/24767 Differential Revision: D15279651 Pulled By: cpojer fbshipit-source-id: 2bbdc4bbcbeae8945588ac5e3e895c49e6ac9e1a --- RNTester/android/app/BUCK | 1 + ReactAndroid/build.gradle | 1 + .../java/com/facebook/react/modules/blob/BUCK | 2 + .../react/modules/blob/BlobCollector.java | 25 ++++++++ .../react/modules/blob/BlobModule.java | 39 +++++++----- .../react/modules/blob/jni/Android.mk | 21 +++++++ .../com/facebook/react/modules/blob/jni/BUCK | 22 +++++++ .../react/modules/blob/jni/BlobCollector.cpp | 61 +++++++++++++++++++ .../react/modules/blob/jni/BlobCollector.h | 39 ++++++++++++ .../react/modules/blob/jni/OnLoad.cpp | 13 ++++ .../src/main/jni/react/jni/Android.mk | 2 + 11 files changed, 211 insertions(+), 15 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.java create mode 100644 ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/Android.mk create mode 100644 ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BUCK create mode 100644 ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.cpp create mode 100644 ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.h create mode 100644 ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/OnLoad.cpp diff --git a/RNTester/android/app/BUCK b/RNTester/android/app/BUCK index ceb05c3e109291..5e839d5e9153ca 100644 --- a/RNTester/android/app/BUCK +++ b/RNTester/android/app/BUCK @@ -25,6 +25,7 @@ rn_android_library( react_native_target("java/com/facebook/react:react"), react_native_target("java/com/facebook/react/common:build_config"), react_native_target("java/com/facebook/react/modules/core:core"), + react_native_target("java/com/facebook/react/views/text:text"), react_native_target("java/com/facebook/react/shell:shell"), react_native_target("jni/prebuilt:android-jsc"), # .so files are prebuilt by Gradle with `./gradlew :ReactAndroid:packageReactNdkLibsForBuck` diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index 6e44769892cbbc..84207fb50578dc 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -222,6 +222,7 @@ def getNdkBuildFullPath() { task buildReactNdkLib(dependsOn: [prepareJSC, 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_PROJECT_PATH=null", diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BUCK b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BUCK index ac8fbcbc9f87ec..72c927348f08e4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BUCK @@ -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"), @@ -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"), ], diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.java b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.java new file mode 100644 index 00000000000000..715f3dd38f6300 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.java @@ -0,0 +1,25 @@ +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(); + synchronized (jsContext) { + nativeInstall(blobModule, jsContext.get()); + } + } + }); + } + + private native static void nativeInstall(Object blobModule, long jsContext); +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java index 830180c6ffc308..7bb5f281e49118 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java @@ -6,13 +6,10 @@ */ package com.facebook.react.modules.blob; -import android.content.ContentResolver; -import android.content.Context; import android.content.res.Resources; import android.database.Cursor; import android.net.Uri; import android.provider.MediaStore; -import androidx.annotation.Nullable; import android.webkit.MimeTypeMap; import com.facebook.react.bridge.Arguments; @@ -40,6 +37,7 @@ import java.util.Map; import java.util.UUID; +import androidx.annotation.Nullable; import okhttp3.MediaType; import okhttp3.RequestBody; import okhttp3.ResponseBody; @@ -151,6 +149,11 @@ public BlobModule(ReactApplicationContext reactContext) { super(reactContext); } + @Override + public void initialize() { + BlobCollector.install(getReactApplicationContext(), this); + } + @Override public String getName() { return NAME; @@ -178,11 +181,15 @@ public String store(byte[] data) { } public void store(byte[] data, String blobId) { - mBlobs.put(blobId, data); + synchronized (mBlobs) { + mBlobs.put(blobId, data); + } } public void remove(String blobId) { - mBlobs.remove(blobId); + synchronized (mBlobs) { + mBlobs.remove(blobId); + } } public @Nullable byte[] resolve(Uri uri) { @@ -201,17 +208,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) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/Android.mk b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/Android.mk new file mode 100644 index 00000000000000..b0ef370c574413 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/Android.mk @@ -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 jscruntime +LOCAL_SHARED_LIBRARIES := libfolly_json libfb libreactnativejni + +include $(BUILD_SHARED_LIBRARY) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BUCK b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BUCK new file mode 100644 index 00000000000000..c02646779b63f8 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BUCK @@ -0,0 +1,22 @@ +load("//tools/build_defs/oss:rn_defs.bzl", "ANDROID", "FBJNI_TARGET", "react_native_target", "react_native_xplat_target", "rn_xplat_cxx_library") + +rn_xplat_cxx_library( + name = "jni", + srcs = glob(["*.cpp"]), + headers = glob(["*.h"]), + header_namespace = "", + compiler_flags = ["-fexceptions"], + fbandroid_allow_jni_merging = True, + platforms = ANDROID, + soname = "libreactnativeblob.$(ext)", + visibility = [ + "PUBLIC", + ], + deps = [ + "fbsource//xplat/folly:molly", + FBJNI_TARGET, + react_native_target("jni/react/jni:jni"), + react_native_xplat_target("jsi:JSCRuntime"), + react_native_xplat_target("jsiexecutor:jsiexecutor"), + ], +) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.cpp b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.cpp new file mode 100644 index 00000000000000..d14d3885253b9d --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.cpp @@ -0,0 +1,61 @@ +// 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 +#include +#include + +using namespace facebook; + +namespace facebook { +namespace react { + +static constexpr auto kBlobModuleJavaDescriptor = + "com/facebook/react/modules/blob/BlobModule"; + +BlobCollector::BlobCollector( + jni::global_ref blobModule, + const std::string &blobId) + : blobModule_(blobModule), blobId_(blobId) {} + +BlobCollector::~BlobCollector() { + auto removeMethod = jni::findClassStatic(kBlobModuleJavaDescriptor) + ->getMethod("remove"); + removeMethod(blobModule_, jni::make_jstring(blobId_).get()); +} + +void BlobCollector::nativeInstall( + jni::alias_ref jThis, + jni::alias_ref 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(blobModuleRef, blobId); + return jsi::Object::createFromHostObject(rt, blobCollector); + })); +} + +void BlobCollector::registerNatives() { + registerHybrid( + {makeNativeMethod("nativeInstall", BlobCollector::nativeInstall)}); +} + +} // namespace react +} // namespace facebook diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.h b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.h new file mode 100644 index 00000000000000..b013cf8bf665c7 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.h @@ -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 +#include + +namespace facebook { +namespace react { + +class BlobCollector : public jni::HybridClass, + public jsi::HostObject { + public: + BlobCollector( + jni::global_ref blobManager, + const std::string &blobId); + ~BlobCollector(); + + static constexpr auto kJavaDescriptor = + "Lcom/facebook/react/modules/blob/BlobCollector;"; + + static void nativeInstall( + jni::alias_ref jThis, + jni::alias_ref blobModule, + jlong jsContextNativePointer); + + static void registerNatives(); + + private: + friend HybridBase; + + jni::global_ref blobModule_; + const std::string blobId_; +}; + +} // namespace react +} // namespace facebook diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/OnLoad.cpp b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/OnLoad.cpp new file mode 100644 index 00000000000000..3707d02665e44a --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/OnLoad.cpp @@ -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 + +#include "BlobCollector.h" + +JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) { + return facebook::jni::initialize( + vm, [] { facebook::react::BlobCollector::registerNatives(); }); +} diff --git a/ReactAndroid/src/main/jni/react/jni/Android.mk b/ReactAndroid/src/main/jni/react/jni/Android.mk index 7d410f44c1078b..1cf03a78a5ea19 100644 --- a/ReactAndroid/src/main/jni/react/jni/Android.mk +++ b/ReactAndroid/src/main/jni/react/jni/Android.mk @@ -68,3 +68,5 @@ include $(REACT_SRC_DIR)/turbomodule/core/jni/Android.mk # $(call import-module,jscexecutor) include $(REACT_SRC_DIR)/jscexecutor/Android.mk + +include $(REACT_SRC_DIR)/modules/blob/jni/Android.mk