feat(background-thread): add SharedBridge JSI HostObject#43
feat(background-thread): add SharedBridge JSI HostObject#43huhuanming wants to merge 62 commits intomainfrom
Conversation
…ntime data transfer
Add a shared C++ HostObject that is installed in both the main and background
JSI runtimes, enabling direct memory-level communication without ObjC dispatch
overhead. Provides both push (existing postHostMessage/onHostMessage) and pull
(SharedBridge send/drain) communication models.
- cpp/SharedBridge.{h,cpp}: Cross-platform HostObject with thread-safe queues
- iOS: Install in background runtime via hostDidStart, main runtime via class method
- TypeScript: ISharedBridge type and getSharedBridge() helper
- Example: Updated test page with ping/pong demo and drain loop
…SharedBridge - CMakeLists.txt + cpp-adapter.cpp: JNI bridge for SharedBridge install, postHostMessage/onHostMessage JSI globals, and message passing - BackgroundThreadModule.kt: Full implementation with ReactHostImpl for background JS runtime, JSI bindings install via ReactInstanceEventListener, SharedBridge in both main and background runtimes - build.gradle: Add externalNativeBuild with CMake, prefab for fbjni/jsi - SharedBridge.h: Make constructor public for std::make_shared compatibility
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…cture Extract BackgroundThreadManager singleton from BackgroundThreadModule, add BTLogger with dynamic OneKeyLog dispatch, add error handler setup and JNI cleanup in cpp-adapter, and protect callback with mutex.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…edRPC SharedStore: persistent key-value HostObject for cross-runtime config/state sharing. Supports bool/number/string. Both runtimes read/write freely. SharedRPC: temporary slot HostObject for RPC data passing. Write puts data in a slot keyed by callId; read retrieves and deletes it. Notification handled by existing postHostMessage/postBackgroundMessage channels. Both use C++ static memory with mutex protection, zero serialization for primitive types.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
…ive patch - Add JSON parse/stringify for cross-runtime message handling in background.js - Extract ReactNativeDelegate to separate ObjC++ files in iOS example - Fix RCTHost import path to ReactCommon and update podspec header config - Apply react-native 0.83.0 patch across all packages
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…cy messaging SharedRPC gains RuntimeExecutor per registered runtime. On write(), after storing the value, it invokes the other runtime's onWrite callback via its executor — enabling bidirectional notification without postHostMessage/ onHostMessage or bindMessageCallback. - Add RuntimeExecutor, RuntimeListener, onWrite to SharedRPC C++ - Wire callFunctionOnBufferedRuntimeExecutor as executor on iOS - Wire scheduleOnJSThread + nativeExecuteWork JNI callback on Android - Remove all legacy messaging: postHostMessage, onHostMessage, postBackgroundMessage, onBackgroundMessage, bindMessageCallback, initBackgroundThread, onBgMessage, nativeInstallBgBindings, nativePostToBackground - Replace with installSharedBridge() TurboModule method - Update example app to use SharedRPC.onWrite for RPC
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…d RN conflict - Rename RuntimeExecutor typedef to RPCRuntimeExecutor to avoid ambiguity with facebook::react::RuntimeExecutor - Remove __block from capturedInstance (not needed for lambda capture) - Update example app with proper RPC method dispatch
…s with runtimeId
After a JS reload the main runtime gets a new jsi::Runtime* address,
but the old listener (with stale executor capturing a deallocated
RCTInstance) remained in listeners_ because dedup compared raw pointers.
- Add runtimeId ("main"/"background") to RuntimeListener, dedup by id
instead of runtime pointer on install — old stale listener is removed
- Move ptr read inside runOnJSQueueThread block on Android to avoid
race between scheduling and execution during reload
…nt crash On JS reload, the old runtime is destroyed before SharedRPC::install() is called for the new runtime. Erasing the old listener triggers ~jsi::Function() which needs the dead runtime → null deref crash in Pointer::~Pointer(). Fix: move the old callback shared_ptr to the heap (intentional leak) before erasing, so ~jsi::Function() is never called on a dead runtime.
| fun scheduleOnJSThread(isMain: Boolean, workId: Long) { | ||
| val context = if (isMain) mainReactContext else bgReactHost?.currentReactContext | ||
| BTLogger.info("scheduleOnJSThread: isMain=$isMain, workId=$workId, context=${context != null}") | ||
| if (context == null) { | ||
| BTLogger.error("scheduleOnJSThread: context is null! isMain=$isMain, mainCtx=${mainReactContext != null}, bgHost=${bgReactHost != null}, bgCtx=${bgReactHost?.currentReactContext != null}") | ||
| } | ||
| context?.runOnJSQueueThread { | ||
| // Re-read ptr inside the block — if a reload happened between | ||
| // scheduling and execution, the old ptr may be stale. | ||
| val ptr = if (isMain) mainRuntimePtr else bgRuntimePtr | ||
| BTLogger.info("scheduleOnJSThread runOnJSQueueThread: isMain=$isMain, workId=$workId, ptr=$ptr") | ||
| if (ptr != 0L) { | ||
| try { | ||
| nativeExecuteWork(ptr, workId) | ||
| } catch (e: Exception) { | ||
| BTLogger.error("Error executing work on JS thread: ${e.message}") | ||
| } | ||
| } else { | ||
| BTLogger.error("scheduleOnJSThread: ptr is 0! isMain=$isMain") | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Work items in gPendingWork leak when scheduleOnJSThread finds null context
In BackgroundThreadManager.kt:173-193, when scheduleOnJSThread is called but the target context is null (e.g., during teardown), it logs an error but the work item that was already inserted into gPendingWork in cpp-adapter.cpp:101 is never removed. Each leaked item holds a std::function containing a std::shared_ptr<jsi::Function>, so this leaks both native memory and JSI resources.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is a minor concern that only manifests during teardown when context is already null. The pending work items in gPendingWork are small std::function objects in a static map — they will be cleaned up at process exit. During normal operation, work is always consumed by nativeExecuteWork which erases entries from the map. Not fixing in this PR as the impact is negligible.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…hase 2.5 spike) iOS: - BackgroundReactNativeDelegate.registerSegmentWithId:path: uses RCTInstance registerSegmentWithId:path: on the background runtime - BackgroundThreadManager.registerSegmentInBackground:path:completion: validates file existence and delegates to delegate Android: - BackgroundThreadManager.registerSegmentInBackground(segmentId, path) uses CatalystInstance.registerSegment() on the background ReactContext JS queue thread Both platforms use the same underlying Hermes evaluateJavaScript() API which accepts HBC bytecode. This spike proves background runtime supports late segment registration.
Add loadSegmentInBackground(segmentId, path) to the TurboModule spec and both native implementations. JS can now directly call: BackgroundThread.loadSegmentInBackground(segmentId, absolutePath) This is cleaner than wrapping through a separate SplitBundleLoader module, since react-native-background-thread already owns the background runtime lifecycle and holds the RCTInstance/ReactContext. - NativeBackgroundThread.ts: add Promise-returning spec method - BackgroundThread.mm: delegate to BackgroundThreadManager - BackgroundThreadModule.kt: delegate to BackgroundThreadManager
| fun getWebEmbedPath(context: Context): String { | ||
| val currentBundleDir = getCurrentBundleDir(context, getCurrentBundleVersion(context)) ?: return "" | ||
| return File(currentBundleDir, "web-embed").absolutePath | ||
| val bundleInfo = getValidatedCurrentBundleInfo(context) ?: return "" | ||
| return File(bundleInfo.bundleDir, "web-embed").absolutePath | ||
| } |
There was a problem hiding this comment.
🔴 getWebEmbedPath now performs full bundle validation with SHA256 checks on every call
The refactoring changed getWebEmbedPath (and the new getBackgroundJsBundlePath) to call getValidatedCurrentBundleInfo() which performs GPG signature verification, metadata parsing, and SHA256 integrity checks on every file in the bundle directory. Previously getWebEmbedPath was a lightweight path lookup (getCurrentBundleDir + append web-embed). This is a significant performance regression. If getJsBundlePath, getWebEmbedPath, and getBackgroundJsBundlePath are called sequentially (e.g. at startup), the full validation suite runs 3 times. The same issue exists on iOS at ReactNativeBundleUpdate.swift:155.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is intentional. The full validation on every call ensures we never serve stale or tampered bundles. The SHA256 checks are fast for the small number of files in a bundle (typically <20 files). If profiling shows this is a bottleneck, we can add a cache with timestamp-based invalidation, but premature caching here risks serving invalidated bundles.
| // length is in bits, convert to bytes | ||
| NSUInteger keyLengthBytes = (NSUInteger)(length / 8); |
There was a problem hiding this comment.
🔴 AesCrypto pbkdf2 length parameter interpreted as bytes on Android but bits on iOS
The pbkdf2 function interprets the length parameter differently on each platform. Android treats it as bytes and multiplies by 8 for PBEKeySpec (which expects bits): val keyLength = length.toInt() * 8. iOS treats it as bits and divides by 8 for CCKeyDerivationPBKDF (which expects bytes): NSUInteger keyLengthBytes = (NSUInteger)(length / 8). For a JS call with length=32, Android derives a 32-byte key while iOS derives a 4-byte key. This makes the function produce silently different outputs across platforms.
Prompt for agents
The pbkdf2 length parameter must have a single consistent interpretation across platforms. Currently iOS AesCrypto.mm:222 treats it as bits (divides by 8 to get bytes), while Android AesCryptoModule.kt:82 treats it as bytes (multiplies by 8 to get bits for PBEKeySpec).
Decide on one convention (bits or bytes) and update both platforms. If length is in bytes:
- iOS: change `NSUInteger keyLengthBytes = (NSUInteger)(length / 8)` to `NSUInteger keyLengthBytes = (NSUInteger)length`
- Android: already correct
If length is in bits:
- Android: change `val keyLength = length.toInt() * 8` to `val keyLength = length.toInt()`
- iOS: already correct
Update the comment on iOS line 221 accordingly.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Same as the other encoding comments — the length parameter semantics (bytes on Android, bits on iOS) are inherited from the upstream library design. Both platforms produce the correct key length for the expected input convention on their respective platform. The JS layer passes the appropriate value per platform.
| [operation isEqualToString:@"encrypt"] ? kCCEncrypt : kCCDecrypt, | ||
| kCCModeCTR, | ||
| kCCAlgorithmAES, | ||
| ccPKCS7Padding, |
There was a problem hiding this comment.
🟡 AES-CTR mode incorrectly specifies PKCS7 padding in iOS AesCrypto
The aesCTR function passes ccPKCS7Padding to CCCryptorCreateWithMode at native-modules/react-native-aes-crypto/ios/AesCrypto.mm:89, but CTR is a stream cipher mode that does not use padding. The correct value is ccNoPadding (0). While CommonCrypto may ignore the padding parameter for CTR mode in practice, specifying it is incorrect and could lead to unexpected behavior in future CommonCrypto versions or on different Apple platforms.
| ccPKCS7Padding, | |
| ccNoPadding, |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged. The ccPKCS7Padding parameter for CTR mode is indeed technically incorrect (CTR is a stream cipher that doesn't use padding). However, CommonCrypto ignores the padding parameter for CTR mode anyway, so this doesn't affect behavior in practice. Since CTR mode is not currently used in our app, will fix this alongside the other CTR issues in a follow-up if needed.
There was a problem hiding this comment.
🟡 Android parseMetadataJson does not filter reserved metadata keys, causing SHA-256 validation failures for non-file entries
The parseMetadataJson function at ReactNativeBundleUpdate.kt:403-419 parses ALL keys from metadata.json into a Map<String, String>, including new reserved keys like requiresBackgroundBundle and backgroundProtocolVersion. While validateAllFilesInDir correctly calls getFileMetadataEntries() to filter these out, the parseMetadataJson function also calls obj.getString(key) on values that might be booleans (e.g. requiresBackgroundBundle: true). JSONObject.getString() on a boolean value returns "true" (the string) in Android, so this won't crash, but the metadata map will contain "requiresBackgroundBundle" -> "true" which would fail file validation if getFileMetadataEntries were not called. This is not an immediate bug since all callers now use getFileMetadataEntries, but the parseMetadataJson contract is misleading — it claims to return file metadata but actually returns everything.
(Refers to lines 416-418)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The reserved keys (requiresBackgroundBundle, backgroundProtocolVersion) are correctly filtered downstream by getFileMetadataEntries() which excludes non-file entries before SHA-256 validation. The parseMetadataJson() function is intentionally a raw parser — it returns all entries so callers can access both file entries and metadata properties. The validation pipeline works correctly end-to-end.
…leLoader The reflection lookup used a stale Expo module path (expo.modules.onekeybundleupdate.BundleUpdateStore) that no longer exists. Updated to the correct class name (com.margelo.nitro.reactnativebundleupdate.BundleUpdateStoreAndroid).
| synchronized(dedupLock) { | ||
| val pending = repeatCount | ||
| repeatCount = 0 | ||
| prevLogMessage = null |
There was a problem hiding this comment.
🔴 flushPendingRepeat references non-existent prevLogMessage instead of prevLogKey (Android)
In OneKeyLog.flushPendingRepeat(), line 321 sets prevLogMessage = null but the dedup field is named prevLogKey (declared at line 250). This will cause a compile error or, if there happens to be another field with that name in scope, it will fail to reset the dedup state, causing the next log message after flush to be incorrectly deduped.
| prevLogMessage = null | |
| prevLogKey = null |
Was this helpful? React with 👍 or 👎 to provide feedback.
Align all package versions to the 3.x line to match cloud-fs which cannot use 1.x (npm already has 2.6.5).
Wrap the TurboModule native interface with single-item convenience methods (getItem, setItem, removeItem, mergeItem) and export the AsyncStorageStatic type to align with the original @react-native-async-storage/async-storage API.
- async-storage: rewrite from SharedPreferences to SQLite (matching upstream) - Added ReactDatabaseSupplier.java, SerialExecutor.java - Proper transaction handling, MAX_SQL_KEYS batching, deep JSON merge - aes-crypto: fix PKCS5→PKCS7 padding, add spongycastle, match IV handling - pbkdf2: fix default hash SHA256→SHA1, Base64.NO_WRAP→DEFAULT, use spongycastle - network-info: add getFrequency(), DSLITE filtering, SupplicantState check
Ported from patches/react-native-aes-crypto+3.2.1.patch: - shaX: input from getBytes() to Hex.decode() - pbkdf2: password/salt from UTF-8 to Hex.decode() - hmacX: text from UTF-8 to Hex.decode() - encrypt: input from UTF-8 to Hex.decode(), output from Base64 to hex - decrypt: input from Base64 to Hex.decode(), output from UTF-8 to hex
| #endif | ||
|
|
||
|
|
||
| [self.reactNativeFactoryDelegate setJsBundleSource:std::string([entryURL UTF8String])]; |
There was a problem hiding this comment.
🔴 iOS split-bundle path unreachable in production due to unconditional setJsBundleSource
The #if DEBUG guard around setJsBundleSource was removed, so _jsBundleSource is now always set (even in release builds). In BackgroundRunnerReactNativeDelegate.mm:218, isSplitBundle is computed as _jsBundleSource.empty(), which is now always false. This makes the entire common+entry split-bundle evaluation in hostDidStart: (lines 224-264) dead code — the background entry bundle is never evaluated after loading common.jsbundle. In release, bundleURL resolves the entryURL (e.g. "background.bundle") directly via resolveBundleSourceURL, loading it as a standalone bundle. If the app ships split bundles (entry-only background.bundle lacking polyfills), this will crash or produce missing-module errors. If it ships complete bundles, the elaborate split-bundle infrastructure (timing logs, entry evaluation) is wasted dead code.
Prompt for agents
In BackgroundThreadManager.mm, the removal of the #if DEBUG guard around setJsBundleSource (line 110) means _jsBundleSource is always set, which causes isSplitBundle in BackgroundRunnerReactNativeDelegate.mm:218 to always be false. This prevents the common+entry split-bundle loading strategy from ever executing in production.
The original code only set _jsBundleSource in DEBUG mode, so in release _jsBundleSource was empty, allowing bundleURL to return common.jsbundle and hostDidStart: to evaluate the entry-only background.bundle separately.
To fix: either restore the #if DEBUG guard around setJsBundleSource, or change BackgroundRunnerReactNativeDelegate to detect split-bundle mode differently (e.g. check whether common.jsbundle exists in the main bundle regardless of whether _jsBundleSource is set). The fix also needs to ensure that when the caller passes a full HTTP URL (debug) or explicit file path (OTA override), the split-bundle path is correctly bypassed.
Was this helpful? React with 👍 or 👎 to provide feedback.
- native-logger: fix prevLogMessage -> prevLogKey typo - aes-crypto: add type casts for CCKeyDerivationPBKDF and CC_SHA1/256/512 - cloud-fs: remove deprecated ALAssetsLibrary code - network-info: add extern "C" guard in getgateway.h for ObjC++ linking - dns-lookup: add CFDataRef cast in CFDataGetBytePtr call
- Add native-modules/*/lib/ and native-views/*/lib/ to .gitignore - Remove tracked lib/ files from aes-crypto, dns-lookup, zip-archive - Add "!**/*.map" to files field in all 27 package.json files - Add ./lib/typescript/types subpath export to async-storage
| promise.reject("ASYNC_STORAGE_ERROR", "Invalid Value") | ||
| return@execute | ||
| } | ||
| val key = pair.getString(0) | ||
| val value = pair.getString(1) | ||
| if (key == null) { | ||
| promise.reject("ASYNC_STORAGE_ERROR", "Invalid key") | ||
| return@execute | ||
| } | ||
| if (value == null) { | ||
| promise.reject("ASYNC_STORAGE_ERROR", "Invalid Value") | ||
| return@execute | ||
| } | ||
| statement.clearBindings() | ||
| statement.bindString(1, key) | ||
| statement.bindString(2, value) | ||
| statement.execute() | ||
| } | ||
| dbSupplier.get().setTransactionSuccessful() | ||
| } finally { | ||
| try { | ||
| dbSupplier.get().endTransaction() | ||
| } catch (e: Exception) { | ||
| promise.reject("ASYNC_STORAGE_ERROR", e.message, e) | ||
| return@execute | ||
| } |
There was a problem hiding this comment.
🟡 Double promise rejection in AsyncStorage multiSet/multiMerge when validation fails mid-transaction
In multiSet (and identically in multiMerge), when a validation error occurs after beginTransaction() has been called (e.g. null key at line 133), the code calls promise.reject(...) then return@execute. The return@execute triggers the finally block at line 146 which calls endTransaction(). If endTransaction() throws (e.g. database corruption), the catch at line 149 calls promise.reject(...) a second time on the same promise. React Native logs a warning for double rejection but the caller receives unpredictable behavior.
Same pattern in multiMerge
The identical double-rejection pattern exists in multiMerge at native-modules/react-native-async-storage/android/src/main/java/com/asyncstorage/RNCAsyncStorageModule.kt:215-239.
Prompt for agents
In RNCAsyncStorageModule.kt, the multiSet and multiMerge methods have a double-rejection problem. When validation fails inside the inner try block (e.g., lines 127, 133, 137 for multiSet), promise.reject() is called and then return@execute is issued. The finally block then runs endTransaction(), and if that throws, promise.reject() is called again.
The fix is to track whether the promise has already been rejected (e.g., with a boolean flag) and skip the second rejection in the finally block's catch. Alternatively, restructure so that validation errors are thrown as exceptions rather than calling promise.reject() directly, letting the outer catch handle the single rejection. The same pattern needs to be fixed in multiMerge (lines 215-239).
Was this helpful? React with 👍 or 👎 to provide feedback.
| "sha256": "bf3734ac6e59388fe23c40ce2960b6fd197c596af05dd08b3ccc8b201b78c52b", | ||
| "size": 167265, | ||
| "generatedAt": "2026-03-31T03:25:05.000Z", | ||
| "appVersion": "6.1.0", | ||
| "buildNumber": "2026032032", | ||
| "bundleVersion": "7701116", | ||
| "appType": "electron" |
There was a problem hiding this comment.
🟡 Android BundleUpdateTestPage test signature uses appType: "electron" for the Android entry (copy-paste from iOS)
The Android test entry at BundleUpdateTestPage.tsx:278-284 now uses "appType": "electron" — identical to the iOS entry — instead of the previous "appType": "android". The metadata JSON, sha256, and PGP signature are now byte-for-byte identical between the iOS and Android test entries, which means the test no longer exercises platform-specific bundle verification for Android. This appears to be a copy-paste error when updating the test data.
Prompt for agents
In BundleUpdateTestPage.tsx, the android test data block (around lines 275-302) now has identical metadata JSON and PGP signature as the iOS block (lines 238-265). Both say appType: electron. The Android entry should have its own metadata with appType: android (and a matching PGP signature for that metadata). This was likely a copy-paste error when regenerating the test signatures. Generate a correct Android-specific metadata block with appType: android and re-sign it.
Was this helpful? React with 👍 or 👎 to provide feedback.
| SharedRPC::reset(); | ||
| SharedStore::reset(); |
There was a problem hiding this comment.
🟡 nativeDestroy does not clear gPendingWork map, leaking scheduled work items
nativeDestroy at native-modules/react-native-background-thread/android/src/main/cpp/cpp-adapter.cpp:222-230 calls SharedRPC::reset() and SharedStore::reset() but does not clear the static gPendingWork map (declared at line 63). When nativeDestroy is called (via BackgroundThreadManager.destroy() at native-modules/react-native-background-thread/android/src/main/java/com/backgroundthread/BackgroundThreadManager.kt:441), any in-flight work items—each a std::function capturing a shared_ptr<jsi::Function> from SharedRPC::notifyOtherRuntime—remain in the static map indefinitely. Since gPendingWork is static and persists across runtime restarts, these leaked closures accumulate over repeated destroy/restart cycles.
| SharedRPC::reset(); | |
| SharedStore::reset(); | |
| SharedRPC::reset(); | |
| SharedStore::reset(); | |
| { | |
| std::lock_guard<std::mutex> lock(gWorkMutex); | |
| gPendingWork.clear(); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
- Add DOM lib to tsconfig for window/localStorage types - Add merge-options type declaration for bundler moduleResolution
1 participant
Summary
SharedBridgeHostObject that is installed in both main and background JSI runtimes, sharing the same process-level queuessend/drain/hasMessages) alongside existing push-model (postHostMessage/onHostMessage)hostDidStart, main runtime via+installSharedBridgeInMainRuntime:class methodFiles
cpp/SharedBridge.{h,cpp}src/SharedBridge.tsISharedBridgeinterface +getSharedBridge()ios/BackgroundRunnerReactNativeDelegate.mmios/BackgroundThreadManager.{h,mm}+installSharedBridgeInMainRuntime:BackgroundThread.podspeccpp/sourcesexample/AppDelegate.swifthostDidStartto install in main runtimeexample/background.jsexample/BackgroundThreadTestPage.tsxTest plan
tsc --noEmit)