Skip to content

Commit

Permalink
Release underlying resources when JS instance is GC'ed on Android
Browse files Browse the repository at this point in the history
Summary:
[Android] [Added] - Release underlying resources when JS instance is GC'ed on Android

D15279651 introduced a crash for Oculus Twilight on Android (T45199437), so it was reverted by D15611385.

This diff fixes the crash and re-applies D15279651. The problem was that ProGuard renamed BlobModule.remove() to BlobModule.release(), but the C++ code in `BlobCollector.cpp` still expected the old name. I confirmed this by looking at the Extracted Symbols file for the build which introduces the crash (https://fburl.com/mobile/ud40od3i):

```
com.facebook.react.modules.blob.BlobModule -> com.facebook.react.modules.blob.BlobModule:
...
8190:8193:void remove(java.lang.String):190:193 -> release
...
```

See the full log file here: https://fburl.com/pn02bwkb.

The solution is to annotate the method with `DoNotStrip` so that ProGuard doesn't rename it.

Reviewed By: mdvacca, cpojer

Differential Revision: D15826082

fbshipit-source-id: f7470d394666cd34c1acae5c6ffaecc84d5ca5a3
  • Loading branch information
makovkastar authored and M-i-k-e-l committed Mar 10, 2020
1 parent ec39c9e commit d49a8c1
Show file tree
Hide file tree
Showing 11 changed files with 213 additions and 15 deletions.
1 change: 1 addition & 0 deletions RNTester/android/app/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
1 change: 1 addition & 0 deletions ReactAndroid/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,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_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,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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@
*/
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.proguard.annotations.DoNotStrip;
import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
Expand All @@ -40,6 +38,7 @@
import java.util.Map;
import java.util.UUID;

import androidx.annotation.Nullable;
import okhttp3.MediaType;
import okhttp3.RequestBody;
import okhttp3.ResponseBody;
Expand Down Expand Up @@ -151,6 +150,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 @@ -178,11 +182,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 @@ -201,17 +210,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 jscruntime
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_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"),
],
)
Original file line number Diff line number Diff line change
@@ -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 <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() {
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(); });
}
2 changes: 2 additions & 0 deletions ReactAndroid/src/main/jni/react/jni/Android.mk
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,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

0 comments on commit d49a8c1

Please sign in to comment.