Skip to content

Commit 5f0c857

Browse files
javachemeta-codesync[bot]
javache
authored and
meta-codesync[bot]
committed
Migrate BlobModule to TurboModuleWithJSIBindings for safe JSI access (#56272)
Summary: Pull Request resolved: #56272 BlobCollector::nativeInstall received a raw jlong pointer to jsi::Runtime from Java and cast it directly, with no thread safety. Migrated BlobModule to implement TurboModuleWithJSIBindings, which provides a safe getBindingsInstaller() callback guaranteed to run on the JS thread during module initialization. Removed the unsafe raw pointer path. Changelog: [Internal] Reviewed By: alanleedev Differential Revision: D98712509
1 parent 4a59f90 commit 5f0c857

File tree

9 files changed

+73
-144
lines changed

9 files changed

+73
-144
lines changed

‎packages/react-native/ReactAndroid/api/ReactAndroid.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2447,16 +2447,16 @@ public abstract interface class com/facebook/react/modules/appregistry/AppRegist
24472447
public abstract fun unmountApplicationComponentAtRootTag (I)V
24482448
}
24492449

2450-
public final class com/facebook/react/modules/blob/BlobModule : com/facebook/fbreact/specs/NativeBlobModuleSpec {
2450+
public final class com/facebook/react/modules/blob/BlobModule : com/facebook/fbreact/specs/NativeBlobModuleSpec, com/facebook/react/turbomodule/core/interfaces/TurboModuleWithJSIBindings {
24512451
public static final field Companion Lcom/facebook/react/modules/blob/BlobModule$Companion;
24522452
public static final field NAME Ljava/lang/String;
24532453
public fun <init> (Lcom/facebook/react/bridge/ReactApplicationContext;)V
24542454
public fun addNetworkingHandler ()V
24552455
public fun addWebSocketHandler (D)V
24562456
public fun createFromParts (Lcom/facebook/react/bridge/ReadableArray;Ljava/lang/String;)V
2457+
public fun getBindingsInstaller ()Lcom/facebook/react/turbomodule/core/interfaces/BindingsInstallerHolder;
24572458
public final fun getLengthOfBlob (Ljava/lang/String;)J
24582459
public fun getTypedExportedConstants ()Ljava/util/Map;
2459-
public fun initialize ()V
24602460
public fun release (Ljava/lang/String;)V
24612461
public final fun remove (Ljava/lang/String;)V
24622462
public fun removeWebSocketHandler (D)V

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.kt‎

Lines changed: 0 additions & 31 deletions
This file was deleted.

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.kt‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import com.facebook.react.bridge.buildReadableMap
2323
import com.facebook.react.module.annotations.ReactModule
2424
import com.facebook.react.modules.network.NetworkingModule
2525
import com.facebook.react.modules.websocket.WebSocketModule
26+
import com.facebook.react.turbomodule.core.interfaces.BindingsInstallerHolder
27+
import com.facebook.react.turbomodule.core.interfaces.TurboModuleWithJSIBindings
28+
import com.facebook.soloader.SoLoader
2629
import java.io.ByteArrayOutputStream
2730
import java.io.File
2831
import java.io.FileNotFoundException
@@ -39,7 +42,7 @@ import okio.ByteString
3942

4043
@ReactModule(name = NativeBlobModuleSpec.NAME)
4144
public class BlobModule(reactContext: ReactApplicationContext) :
42-
NativeBlobModuleSpec(reactContext) {
45+
NativeBlobModuleSpec(reactContext), TurboModuleWithJSIBindings {
4346

4447
private val blobs = HashMap<String, ByteArray>()
4548

@@ -128,10 +131,6 @@ public class BlobModule(reactContext: ReactApplicationContext) :
128131
}
129132
}
130133

131-
public override fun initialize() {
132-
BlobCollector.install(reactApplicationContext, this)
133-
}
134-
135134
public override fun getTypedExportedConstants(): Map<String, Any> {
136135
val resources = reactApplicationContext.resources
137136
val packageName = reactApplicationContext.packageName
@@ -331,7 +330,13 @@ public class BlobModule(reactContext: ReactApplicationContext) :
331330
remove(blobId)
332331
}
333332

333+
@DoNotStrip external override fun getBindingsInstaller(): BindingsInstallerHolder
334+
334335
public companion object {
336+
init {
337+
SoLoader.loadLibrary("reactnativeblob")
338+
}
339+
335340
public const val NAME: String = NativeBlobModuleSpec.NAME
336341
}
337342
}

‎packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp‎

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -40,38 +40,45 @@ size_t BlobCollector::getBlobLength() {
4040
return static_cast<size_t>(length);
4141
}
4242

43-
void BlobCollector::nativeInstall(
44-
jni::alias_ref<jclass> /*unused*/,
45-
jni::alias_ref<jobject> blobModule,
46-
jlong jsContextNativePointer) {
47-
auto& runtime = *((jsi::Runtime*)jsContextNativePointer);
48-
auto blobModuleRef = jni::make_global(blobModule);
49-
runtime.global().setProperty(
50-
runtime,
51-
"__blobCollectorProvider",
52-
jsi::Function::createFromHostFunction(
53-
runtime,
54-
jsi::PropNameID::forAscii(runtime, "__blobCollectorProvider"),
55-
1,
56-
[blobModuleRef](
57-
jsi::Runtime& rt,
58-
const jsi::Value& /*thisVal*/,
59-
const jsi::Value* args,
60-
size_t /*count*/) {
61-
auto blobId = args[0].asString(rt).utf8(rt);
62-
auto blobCollector =
63-
std::make_shared<BlobCollector>(blobModuleRef, blobId);
64-
auto blobCollectorJsObject =
65-
jsi::Object::createFromHostObject(rt, blobCollector);
66-
blobCollectorJsObject.setExternalMemoryPressure(
67-
rt, blobCollector->getBlobLength());
68-
return blobCollectorJsObject;
69-
}));
43+
// static
44+
void BlobModuleJSIBindings::registerNatives() {
45+
javaClassLocal()->registerNatives({
46+
makeNativeMethod(
47+
"getBindingsInstaller", BlobModuleJSIBindings::getBindingsInstaller),
48+
});
7049
}
7150

72-
void BlobCollector::registerNatives() {
73-
registerHybrid(
74-
{makeNativeMethod("nativeInstall", BlobCollector::nativeInstall)});
51+
// static
52+
jni::local_ref<BindingsInstallerHolder::javaobject>
53+
BlobModuleJSIBindings::getBindingsInstaller(
54+
jni::alias_ref<BlobModuleJSIBindings> jobj) {
55+
auto blobModuleRef = jni::make_global(jobj);
56+
return BindingsInstallerHolder::newObjectCxxArgs(
57+
[blobModuleRef = std::move(blobModuleRef)](
58+
jsi::Runtime& runtime, const std::shared_ptr<CallInvoker>&) {
59+
runtime.global().setProperty(
60+
runtime,
61+
"__blobCollectorProvider",
62+
jsi::Function::createFromHostFunction(
63+
runtime,
64+
jsi::PropNameID::forAscii(runtime, "__blobCollectorProvider"),
65+
1,
66+
[blobModuleRef](
67+
jsi::Runtime& rt,
68+
const jsi::Value& /*thisVal*/,
69+
const jsi::Value* args,
70+
size_t /*count*/) {
71+
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
72+
auto blobId = args[0].asString(rt).utf8(rt);
73+
auto blobCollector =
74+
std::make_shared<BlobCollector>(blobModuleRef, blobId);
75+
auto blobCollectorJsObject =
76+
jsi::Object::createFromHostObject(rt, blobCollector);
77+
blobCollectorJsObject.setExternalMemoryPressure(
78+
rt, blobCollector->getBlobLength());
79+
return blobCollectorJsObject;
80+
}));
81+
});
7582
}
7683

7784
} // namespace facebook::react

‎packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,33 @@
77

88
#pragma once
99

10+
#include <ReactCommon/BindingsInstallerHolder.h>
1011
#include <fbjni/fbjni.h>
1112
#include <jsi/jsi.h>
1213

1314
namespace facebook::react {
1415

15-
class BlobCollector : public jni::HybridClass<BlobCollector>, public jsi::HostObject {
16+
class BlobCollector : public jsi::HostObject {
1617
public:
1718
BlobCollector(jni::global_ref<jobject> blobModule, std::string blobId);
1819
~BlobCollector();
1920

2021
size_t getBlobLength();
2122

22-
static constexpr auto kJavaDescriptor = "Lcom/facebook/react/modules/blob/BlobCollector;";
23+
private:
24+
jni::global_ref<jobject> blobModule_;
25+
const std::string blobId_;
26+
};
2327

24-
static void
25-
nativeInstall(jni::alias_ref<jclass> /*unused*/, jni::alias_ref<jobject> blobModule, jlong jsContextNativePointer);
28+
class BlobModuleJSIBindings : public jni::JavaClass<BlobModuleJSIBindings> {
29+
public:
30+
static constexpr const char *kJavaDescriptor = "Lcom/facebook/react/modules/blob/BlobModule;";
2631

2732
static void registerNatives();
2833

2934
private:
30-
friend HybridBase;
31-
32-
jni::global_ref<jobject> blobModule_;
33-
const std::string blobId_;
35+
static jni::local_ref<BindingsInstallerHolder::javaobject> getBindingsInstaller(
36+
jni::alias_ref<BlobModuleJSIBindings> jobj);
3437
};
3538

3639
} // namespace facebook::react

‎packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ target_link_libraries(reactnativeblob
2121
fbjni
2222
folly_runtime
2323
jsi
24-
reactnativejni)
24+
reactnativejni
25+
turbomodulejsijni)
2526

2627
target_compile_reactnative_options(reactnativeblob PRIVATE)
2728
target_compile_options(reactnativeblob PRIVATE -fvisibility=hidden)

‎packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/OnLoad.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@
1111

1212
JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* /*unused*/) {
1313
return facebook::jni::initialize(
14-
vm, [] { facebook::react::BlobCollector::registerNatives(); });
14+
vm, [] { facebook::react::BlobModuleJSIBindings::registerNatives(); });
1515
}

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobCollectorTest.kt‎

Lines changed: 0 additions & 66 deletions
This file was deleted.

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobModuleTest.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import com.facebook.react.bridge.JavaOnlyArray
1212
import com.facebook.react.bridge.JavaOnlyMap
1313
import com.facebook.react.bridge.ReactApplicationContext
1414
import com.facebook.react.bridge.ReactTestHelper
15+
import com.facebook.soloader.SoLoader
1516
import com.facebook.testutils.shadows.ShadowArguments
1617
import java.io.ByteArrayInputStream
1718
import java.nio.ByteBuffer
@@ -22,6 +23,8 @@ import org.junit.After
2223
import org.junit.Before
2324
import org.junit.Test
2425
import org.junit.runner.RunWith
26+
import org.mockito.MockedStatic
27+
import org.mockito.Mockito.mockStatic
2528
import org.robolectric.RobolectricTestRunner
2629
import org.robolectric.Shadows.shadowOf
2730
import org.robolectric.annotation.Config
@@ -33,9 +36,15 @@ class BlobModuleTest {
3336
private lateinit var blobId: String
3437
private lateinit var context: ReactApplicationContext
3538
private lateinit var blobModule: BlobModule
39+
private lateinit var mockedStaticSoLoader: MockedStatic<SoLoader>
3640

3741
@Before
3842
fun prepareModules() {
43+
mockedStaticSoLoader = mockStatic(SoLoader::class.java)
44+
mockedStaticSoLoader
45+
.`when`<Boolean> { SoLoader.loadLibrary("reactnativeblob") }
46+
.thenReturn(true)
47+
3948
bytes = ByteArray(120)
4049
Random.Default.nextBytes(bytes)
4150

@@ -47,6 +56,7 @@ class BlobModuleTest {
4756
@After
4857
fun cleanUp() {
4958
blobModule.remove(blobId)
59+
mockedStaticSoLoader.close()
5060
}
5161

5262
@Test

0 commit comments

Comments
 (0)