Harden RCTArrayBuffer zero-copy conversion for ObjC TurboModules (#57983) - #57983
Open
christophpurrer wants to merge 4 commits into
Open
Harden RCTArrayBuffer zero-copy conversion for ObjC TurboModules (#57983)#57983christophpurrer wants to merge 4 commits into
christophpurrer wants to merge 4 commits into
Conversation
…oad the correct commit data. Differential Revision: D115755247
…57982) Summary: Pull Request resolved: react#57982 Changelog: [ANDROID][FIXED] Enforce the ArrayBuffer borrow contract for Java TurboModules Follow-up hardening for the Android `ArrayBuffer` TurboModule type. Three problems: **1. Borrowed JS-heap bytes outlived the call that lent them.** For a synchronous method, `convertJSIArgsToJNIArgs` hands the module a `ByteBuffer` aliasing the JS `ArrayBuffer`'s bytes without copying. Nothing stopped a module from stashing that `ArrayBuffer` in a field and reading it later, after the JS heap may have moved, freed, or reused the memory — a use-after-free that reads as intermittent data corruption rather than a crash. The borrow is now explicitly scoped to the call frame. `JNIArgs` records every borrowed `ArrayBuffer` and revokes it in its destructor — including when the call throws — via the new `JArrayBuffer::invalidate`, which drops the C++ side's reference to the bytes. `ArrayBuffer.bytes` and `ArrayBuffer.size` then throw, with a message pointing at `ArrayBuffer.arrayBufferWithCopiedBytes`, and `JArrayBuffer::toJSBuffer` throws rather than aliasing revoked memory. Modules that need the bytes past the call copy them; modules that don't keep the zero-copy fast path. Revocation lives entirely on the C++ side: the peer is the single source of truth, and Kotlin asks it through `isBytesValid`. The destructor runs while the stack unwinds, possibly with a Java exception pending, so it resolves each peer pointer at borrow time — the `global_ref` alongside it keeps the Java object, and therefore the peer, alive — and calls only the `noexcept` `JArrayBuffer::invalidate`. No JNI calls are made from the destructor, which is what lets it stay `noexcept` honestly. **2. Argument conversion aborted under runtimes that refuse `tryGetMutableBuffer`.** `jsi::Runtime::tryGetMutableBuffer` and `detached` are not universally implemented: tracing and replay runtimes throw from `tryGetMutableBuffer`, and `detached` throws a `JSINativeException` if the JS-side property isn't a bool. `ArrayBuffer` argument conversion is not wrapped in a try/catch, so either throw propagated out of a JNI frame. Both calls now go through exception-tolerant helpers in `react/bridging/ArrayBuffer.h`; a runtime that refuses to answer is treated as "no native buffer available", which selects the copy path. Routing `AsyncArrayBuffer::acquire` and `::borrow` through the same helper fixes the identical latent bug on the shared C++/ObjC path. **3. A wrong return type from a module crashed instead of raising a JS error.** The `ArrayBufferKind` return path cast the returned `jobject` to `JArrayBuffer` unconditionally. A module returning any other object type produced undefined behavior. The cast is now guarded by an `isInstanceOf` check that throws a `jsi::JSError` naming the offending module and method. Also in this change: - `JByteBufferMutableBuffer::data()` reports null for a zero-capacity direct buffer instead of calling `getDirectBytes()`, which throws for one. That made `createArrayBuffer` throw for an empty `ArrayBuffer`. - Dropped two dead zero-size branches in `JArrayBuffer`: `JByteBuffer::wrapBytes` already routes `size == 0` to an empty buffer. - `JArrayBuffer.cpp` reuses the shared `detail::OwnedBytesBuffer` from `react/bridging/ArrayBuffer.h` instead of a second local copy. - `ArrayBuffer.kt` KDoc corrected: the returned JS `ArrayBuffer` is a new object over the same bytes rather than the identical one, `size` is the capacity and not a view's remaining bytes, and `arrayBufferWithOwnedBytes` documents the caller's lifetime obligation. - `ArrayBuffer.kt` moves from the `bridge` target to `native-types`, alongside the other JNI-backed bridge types. Changelog: [Android][Breaking] - TurboModule methods taking or returning an `ArrayBuffer` now use `com.facebook.react.bridge.ArrayBuffer` instead of `java.nio.ByteBuffer`, and an `ArrayBuffer` argument must not be retained past the method that receives it unless its bytes are copied with `ArrayBuffer.arrayBufferWithCopiedBytes()`. Reviewed By: javache Differential Revision: D115794808
Summary: iOS TurboModules mapped a JS `ArrayBuffer` to `NSData` on arguments and `NSMutableData` on returns, so every crossing copied — and `NSMutableData` cannot alias foreign memory, so there was no way to express "these bytes live somewhere else". This adds `RCTArrayBuffer` (`packages/react-native/React/Base/`) as the ObjC representation of an `ArrayBuffer`. It carries an `isOwningBytes` flag: an owning buffer can be stored and read from any thread, a non-owning one aliases bytes valid only for the synchronous call that produced it. Codegen now emits `RCTArrayBuffer *` for `ArrayBufferTypeAnnotation` params (was `NSData *`) and returns (was `NSMutableData *`). ## Changelog: [IOS] [BREAKING] - Add `RCTArrayBuffer`, the ObjC representation of a JS `ArrayBuffer` for TurboModules, with an explicit byte-ownership contract Pull Request resolved: react#57879 Test Plan: - `RCTTurboModuleArrayBufferTests` — 9 tests over the sync in-place path, `isOwningBytes` on a sync argument, returning one's own argument, the void/Promise copy paths, nesting, and a zero-length round trip. - `RCTTurboModuleTests.mm` adds `testNativeBackedArrayBufferIsAliasedAndKeepsBackingStoreAlive`. - `RCTSampleTurboModule` doubles its argument in place and returns the same buffer, covering the path end to end. - Codegen and C++ API snapshots regenerated. Differential Revision: D115629409
|
@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating Diff in D115767439. |
christophpurrer
added a commit
to christophpurrer/react-native-macos
that referenced
this pull request
Aug 17, 2026
…ct#57983) Summary: Pull Request resolved: react#57983 Follow-ups on D115629409, all in the ObjC ArrayBuffer conversion path. Correctness: - Copy JS-heap `ArrayBuffer` arguments whenever a method also takes a function argument. The block the module receives can be invoked after the call returns and can carry the buffer with it, so `mustCopyJSHeapArrayBufferBytes` now inspects the argument list, not just the return kind. - Thread `mustCopyBytes` through `convertJSIArrayToNSArray` and `convertJSIObjectToNSDictionary`, so buffers nested inside array/object arguments follow the same aliasing rules as top-level ones. They were previously always aliased. - Guard `tryGetMutableBuffer` with try/catch: `TracingRuntime` throws `std::logic_error` instead of returning null. Falling through to the JS-heap rules is correct for any buffer; the unsupported-runtime warning is logged once. - Reject detached buffers via `detail::throwIfDetached` (extracted from `AsyncArrayBuffer` so both conversion paths share one implementation) before reading `data()`/`size()`. - Convert `NSException` raised during argument conversion into a `JSError`. Argument conversion runs on the JS thread, outside the `try` in `performMethodInvocation`, and `RCTArrayBuffer` can raise (`NSInvalidArgumentException`, `NSMallocException`). - Run the caller`s `cleanup` block before `-initWithBytesNoCopy:...` raises on a NULL/non-zero-length mismatch; nothing else would ever release those bytes. - Normalize `mutableBytes` to NULL for zero-length buffers in the designated initializer, so the documented "NULL iff empty" invariant holds for every factory. - Log an `RCTLogError` when `convertObjCObjectToJSIValue` falls through to `undefined`, instead of silently handing JS `undefined` for an unsupported ObjC class. Build fix: - Migrate the macOS sample TurboModule to `RCTArrayBuffer`. The iOS sample was migrated in D115629409 but the macOS one still used `NSData`/`NSMutableData`, which no longer matches the generated spec, breaking the internal macOS build. Changelog: [iOS][Fixed] Harden RCTArrayBuffer zero-copy conversion for ObjC TurboModules Reviewed By: cipolleschi Differential Revision: D115767439
christophpurrer
force-pushed
the
export-D115767439
branch
from
August 17, 2026 20:34
522a7d8 to
8035c85
Compare
…ct#57983) Summary: Pull Request resolved: react#57983 Follow-ups on D115629409, all in the ObjC ArrayBuffer conversion path. Correctness: - Copy JS-heap `ArrayBuffer` arguments whenever a method also takes a function argument. The block the module receives can be invoked after the call returns and can carry the buffer with it, so `mustCopyJSHeapArrayBufferBytes` now inspects the argument list, not just the return kind. - Thread `mustCopyBytes` through `convertJSIArrayToNSArray` and `convertJSIObjectToNSDictionary`, so buffers nested inside array/object arguments follow the same aliasing rules as top-level ones. They were previously always aliased. - Guard `tryGetMutableBuffer` with try/catch: `TracingRuntime` throws `std::logic_error` instead of returning null. Falling through to the JS-heap rules is correct for any buffer; the unsupported-runtime warning is logged once. - Reject detached buffers via `detail::throwIfDetached` (extracted from `AsyncArrayBuffer` so both conversion paths share one implementation) before reading `data()`/`size()`. - Convert `NSException` raised during argument conversion into a `JSError`. Argument conversion runs on the JS thread, outside the `try` in `performMethodInvocation`, and `RCTArrayBuffer` can raise (`NSInvalidArgumentException`, `NSMallocException`). - Run the caller`s `cleanup` block before `-initWithBytesNoCopy:...` raises on a NULL/non-zero-length mismatch; nothing else would ever release those bytes. - Normalize `mutableBytes` to NULL for zero-length buffers in the designated initializer, so the documented "NULL iff empty" invariant holds for every factory. - Log an `RCTLogError` when `convertObjCObjectToJSIValue` falls through to `undefined`, instead of silently handing JS `undefined` for an unsupported ObjC class. Build fix: - Migrate the macOS sample TurboModule to `RCTArrayBuffer`. The iOS sample was migrated in D115629409 but the macOS one still used `NSData`/`NSMutableData`, which no longer matches the generated spec, breaking the internal macOS build. Changelog: [iOS][Fixed] Harden RCTArrayBuffer zero-copy conversion for ObjC TurboModules Differential Revision: D115767439
christophpurrer
force-pushed
the
export-D115767439
branch
from
August 17, 2026 22:08
8035c85 to
9e9d977
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Follow-ups on D115629409, all in the ObjC ArrayBuffer conversion path.
Correctness:
ArrayBufferarguments whenever a method also takes a functionargument. The block the module receives can be invoked after the call returns
and can carry the buffer with it, so
mustCopyJSHeapArrayBufferBytesnowinspects the argument list, not just the return kind.
mustCopyBytesthroughconvertJSIArrayToNSArrayandconvertJSIObjectToNSDictionary, so buffers nested inside array/objectarguments follow the same aliasing rules as top-level ones. They were
previously always aliased.
tryGetMutableBufferwith try/catch:TracingRuntimethrowsstd::logic_errorinstead of returning null. Falling through to the JS-heaprules is correct for any buffer; the unsupported-runtime warning is logged once.
detail::throwIfDetached(extracted fromAsyncArrayBufferso both conversion paths share one implementation) beforereading
data()/size().NSExceptionraised during argument conversion into aJSError.Argument conversion runs on the JS thread, outside the
tryinperformMethodInvocation, andRCTArrayBuffercan raise(
NSInvalidArgumentException,NSMallocException).scleanupblock before-initWithBytesNoCopy:...` raises on aNULL/non-zero-length mismatch; nothing else would ever release those bytes.
mutableBytesto NULL for zero-length buffers in the designatedinitializer, so the documented "NULL iff empty" invariant holds for every
factory.
RCTLogErrorwhenconvertObjCObjectToJSIValuefalls through toundefined, instead of silently handing JSundefinedfor an unsupportedObjC class.
Build fix:
RCTArrayBuffer. The iOS sample wasmigrated in D115629409 but the macOS one still used
NSData/NSMutableData,which no longer matches the generated spec, breaking the internal macOS build.
Changelog: [iOS][Fixed] Harden RCTArrayBuffer zero-copy conversion for ObjC TurboModules
Differential Revision: D115767439