From 8345c0b2f8a6f63464144f90ca0bf1012344db42 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Mon, 30 Mar 2026 09:27:00 -0700 Subject: [PATCH] Migrate BlobModule to TurboModuleWithJSIBindings for safe JSI access Summary: 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. Differential Revision: D98712509 --- .../ReactAndroid/api/ReactAndroid.api | 4 +- .../react/modules/blob/BlobCollector.kt | 31 --------- .../facebook/react/modules/blob/BlobModule.kt | 15 +++-- .../react/reactnativeblob/BlobCollector.cpp | 67 ++++++++++--------- .../jni/react/reactnativeblob/BlobCollector.h | 19 +++--- .../jni/react/reactnativeblob/CMakeLists.txt | 3 +- .../main/jni/react/reactnativeblob/OnLoad.cpp | 2 +- .../react/modules/blob/BlobCollectorTest.kt | 66 ------------------ .../react/modules/blob/BlobModuleTest.kt | 17 +++++ 9 files changed, 80 insertions(+), 144 deletions(-) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.kt delete mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobCollectorTest.kt diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 8871af16dbc1..a3232395f1cf 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -2447,16 +2447,16 @@ public abstract interface class com/facebook/react/modules/appregistry/AppRegist public abstract fun unmountApplicationComponentAtRootTag (I)V } -public final class com/facebook/react/modules/blob/BlobModule : com/facebook/fbreact/specs/NativeBlobModuleSpec { +public final class com/facebook/react/modules/blob/BlobModule : com/facebook/fbreact/specs/NativeBlobModuleSpec, com/facebook/react/turbomodule/core/interfaces/TurboModuleWithJSIBindings { public static final field Companion Lcom/facebook/react/modules/blob/BlobModule$Companion; public static final field NAME Ljava/lang/String; public fun (Lcom/facebook/react/bridge/ReactApplicationContext;)V public fun addNetworkingHandler ()V public fun addWebSocketHandler (D)V public fun createFromParts (Lcom/facebook/react/bridge/ReadableArray;Ljava/lang/String;)V + public fun getBindingsInstaller ()Lcom/facebook/react/turbomodule/core/interfaces/BindingsInstallerHolder; public final fun getLengthOfBlob (Ljava/lang/String;)J public fun getTypedExportedConstants ()Ljava/util/Map; - public fun initialize ()V public fun release (Ljava/lang/String;)V public final fun remove (Ljava/lang/String;)V public fun removeWebSocketHandler (D)V diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.kt deleted file mode 100644 index f7108095bc91..000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.kt +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.modules.blob - -import com.facebook.react.bridge.JavaScriptContextHolder -import com.facebook.react.bridge.ReactContext -import com.facebook.soloader.SoLoader - -internal object BlobCollector { - init { - SoLoader.loadLibrary("reactnativeblob") - } - - @JvmStatic - fun install(reactContext: ReactContext, blobModule: BlobModule) { - reactContext.runOnJSQueueThread { - val jsContext: JavaScriptContextHolder? = reactContext.getJavaScriptContextHolder() - // When debugging in chrome the JS context is not available. - if (jsContext != null && jsContext.get() != 0L) { - nativeInstall(blobModule, jsContext.get()) - } - } - } - - private external fun nativeInstall(blobModule: Any, jsContext: Long) -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.kt index ead05404002b..1411aeadf657 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.kt @@ -23,6 +23,9 @@ import com.facebook.react.bridge.buildReadableMap import com.facebook.react.module.annotations.ReactModule import com.facebook.react.modules.network.NetworkingModule import com.facebook.react.modules.websocket.WebSocketModule +import com.facebook.react.turbomodule.core.interfaces.BindingsInstallerHolder +import com.facebook.react.turbomodule.core.interfaces.TurboModuleWithJSIBindings +import com.facebook.soloader.SoLoader import java.io.ByteArrayOutputStream import java.io.File import java.io.FileNotFoundException @@ -39,7 +42,7 @@ import okio.ByteString @ReactModule(name = NativeBlobModuleSpec.NAME) public class BlobModule(reactContext: ReactApplicationContext) : - NativeBlobModuleSpec(reactContext) { + NativeBlobModuleSpec(reactContext), TurboModuleWithJSIBindings { private val blobs = HashMap() @@ -128,10 +131,6 @@ public class BlobModule(reactContext: ReactApplicationContext) : } } - public override fun initialize() { - BlobCollector.install(reactApplicationContext, this) - } - public override fun getTypedExportedConstants(): Map { val resources = reactApplicationContext.resources val packageName = reactApplicationContext.packageName @@ -331,7 +330,13 @@ public class BlobModule(reactContext: ReactApplicationContext) : remove(blobId) } + @DoNotStrip external override fun getBindingsInstaller(): BindingsInstallerHolder + public companion object { + init { + SoLoader.loadLibrary("reactnativeblob") + } + public const val NAME: String = NativeBlobModuleSpec.NAME } } diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp index 40ddf7b7f9c0..d37c05a3aafe 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp @@ -40,38 +40,45 @@ size_t BlobCollector::getBlobLength() { return static_cast(length); } -void BlobCollector::nativeInstall( - jni::alias_ref /*unused*/, - 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); - auto blobCollectorJsObject = - jsi::Object::createFromHostObject(rt, blobCollector); - blobCollectorJsObject.setExternalMemoryPressure( - rt, blobCollector->getBlobLength()); - return blobCollectorJsObject; - })); +// static +void BlobModuleJSIBindings::registerNatives() { + javaClassLocal()->registerNatives({ + makeNativeMethod( + "getBindingsInstaller", BlobModuleJSIBindings::getBindingsInstaller), + }); } -void BlobCollector::registerNatives() { - registerHybrid( - {makeNativeMethod("nativeInstall", BlobCollector::nativeInstall)}); +// static +jni::local_ref +BlobModuleJSIBindings::getBindingsInstaller( + jni::alias_ref jobj) { + auto blobModuleRef = jni::make_global(jobj); + return BindingsInstallerHolder::newObjectCxxArgs( + [blobModuleRef = std::move(blobModuleRef)]( + jsi::Runtime& runtime, const std::shared_ptr&) { + 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*/) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto blobId = args[0].asString(rt).utf8(rt); + auto blobCollector = + std::make_shared(blobModuleRef, blobId); + auto blobCollectorJsObject = + jsi::Object::createFromHostObject(rt, blobCollector); + blobCollectorJsObject.setExternalMemoryPressure( + rt, blobCollector->getBlobLength()); + return blobCollectorJsObject; + })); + }); } } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h index 440e725e28d8..63d07b975c6f 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h @@ -7,30 +7,33 @@ #pragma once +#include #include #include namespace facebook::react { -class BlobCollector : public jni::HybridClass, public jsi::HostObject { +class BlobCollector : public jsi::HostObject { public: BlobCollector(jni::global_ref blobModule, std::string blobId); ~BlobCollector(); size_t getBlobLength(); - static constexpr auto kJavaDescriptor = "Lcom/facebook/react/modules/blob/BlobCollector;"; + private: + jni::global_ref blobModule_; + const std::string blobId_; +}; - static void - nativeInstall(jni::alias_ref /*unused*/, jni::alias_ref blobModule, jlong jsContextNativePointer); +class BlobModuleJSIBindings : public jni::JavaClass { + public: + static constexpr const char *kJavaDescriptor = "Lcom/facebook/react/modules/blob/BlobModule;"; static void registerNatives(); private: - friend HybridBase; - - jni::global_ref blobModule_; - const std::string blobId_; + static jni::local_ref getBindingsInstaller( + jni::alias_ref jobj); }; } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/CMakeLists.txt b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/CMakeLists.txt index cb96bc65fabc..491a3216c7e5 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/CMakeLists.txt +++ b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/CMakeLists.txt @@ -21,7 +21,8 @@ target_link_libraries(reactnativeblob fbjni folly_runtime jsi - reactnativejni) + reactnativejni + turbomodulejsijni) target_compile_reactnative_options(reactnativeblob PRIVATE) target_compile_options(reactnativeblob PRIVATE -fvisibility=hidden) diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/OnLoad.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/OnLoad.cpp index 45f628d2de56..a9391eb0cef9 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/OnLoad.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/OnLoad.cpp @@ -11,5 +11,5 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* /*unused*/) { return facebook::jni::initialize( - vm, [] { facebook::react::BlobCollector::registerNatives(); }); + vm, [] { facebook::react::BlobModuleJSIBindings::registerNatives(); }); } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobCollectorTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobCollectorTest.kt deleted file mode 100644 index 87e7b1e231a3..000000000000 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobCollectorTest.kt +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.modules.blob - -import com.facebook.react.bridge.JavaScriptContextHolder -import com.facebook.react.bridge.ReactContext -import com.facebook.soloader.SoLoader -import org.junit.After -import org.junit.Before -import org.junit.Test -import org.mockito.MockedStatic -import org.mockito.Mockito.mockStatic -import org.mockito.kotlin.any -import org.mockito.kotlin.mock -import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever - -class BlobCollectorTest { - private lateinit var reactContext: ReactContext - private lateinit var mockedStaticSoLoader: MockedStatic - - @Before - fun setUp() { - reactContext = mock() - - mockedStaticSoLoader = mockStatic(SoLoader::class.java) - mockedStaticSoLoader - .`when` { SoLoader.loadLibrary("reactnativeblob") } - .thenReturn(true) - } - - @After - fun tearDown() { - mockedStaticSoLoader.close() - } - - @Test - fun testInstallWithValidJsContext() { - val jsContextHolder = mock() - val jsContext = 1234L - - whenever(reactContext.getJavaScriptContextHolder()).thenReturn(jsContextHolder) - whenever(jsContextHolder.get()).thenReturn(jsContext) - - BlobCollector.install(reactContext, mock()) - - verify(reactContext).runOnJSQueueThread(any()) - } - - @Test - fun testInstallWithInvalidOrNullJsContext() { - val jsContextHolder = mock() - - whenever(reactContext.getJavaScriptContextHolder()).thenReturn(jsContextHolder) - whenever(jsContextHolder.get()).thenReturn(0L) - - BlobCollector.install(reactContext, mock()) - - verify(reactContext).runOnJSQueueThread(any()) - } -} diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobModuleTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobModuleTest.kt index 0cbdab2bc2f7..ca9463f62959 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobModuleTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobModuleTest.kt @@ -12,6 +12,8 @@ import com.facebook.react.bridge.JavaOnlyArray import com.facebook.react.bridge.JavaOnlyMap import com.facebook.react.bridge.ReactApplicationContext import com.facebook.react.bridge.ReactTestHelper +import com.facebook.react.turbomodule.core.interfaces.TurboModuleWithJSIBindings +import com.facebook.soloader.SoLoader import com.facebook.testutils.shadows.ShadowArguments import java.io.ByteArrayInputStream import java.nio.ByteBuffer @@ -19,9 +21,12 @@ import java.util.UUID import kotlin.random.Random import org.assertj.core.api.Assertions.assertThat import org.junit.After +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.MockedStatic +import org.mockito.Mockito.mockStatic import org.robolectric.RobolectricTestRunner import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config @@ -33,9 +38,15 @@ class BlobModuleTest { private lateinit var blobId: String private lateinit var context: ReactApplicationContext private lateinit var blobModule: BlobModule + private lateinit var mockedStaticSoLoader: MockedStatic @Before fun prepareModules() { + mockedStaticSoLoader = mockStatic(SoLoader::class.java) + mockedStaticSoLoader + .`when` { SoLoader.loadLibrary("reactnativeblob") } + .thenReturn(true) + bytes = ByteArray(120) Random.Default.nextBytes(bytes) @@ -47,6 +58,7 @@ class BlobModuleTest { @After fun cleanUp() { blobModule.remove(blobId) + mockedStaticSoLoader.close() } @Test @@ -214,4 +226,9 @@ class BlobModuleTest { assertThat(blob.getInt("size")).isEqualTo(testData.size) assertThat(blob.getString("blobId")).isNotEmpty() } + + @Test + fun testBlobModuleImplementsTurboModuleWithJSIBindings() { + assertTrue(blobModule is TurboModuleWithJSIBindings) + } }