Deduplicate the ArrayBuffer helpers and align the ObjC, C++ and JNI implementations (#58004) - #58004
Open
christophpurrer wants to merge 2 commits into
Open
Deduplicate the ArrayBuffer helpers and align the ObjC, C++ and JNI implementations (#58004)#58004christophpurrer wants to merge 2 commits into
christophpurrer wants to merge 2 commits into
Conversation
|
@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating Diff in D116358593. |
…ock (react#57983) Summary: A JS-heap `ArrayBuffer` argument was aliased, not copied, whenever the method's return kind let the call complete synchronously. That is unsafe if the method also takes a block: the module can hand the buffer to the callback, and JS drains it after the call has returned, by which point the bytes may be gone. The copy decision now also looks at the ObjC method signature, and copies whenever a parameter is a block. Changelog: [iOS][Fixed] - Copy ArrayBuffer arguments in ObjC TurboModules when the method also takes a callback Differential Revision: D115767439
…mplementations (react#58004) Summary: Follow-up cleanup on the `ArrayBuffer` TurboModule plumbing. No behaviour change except the leak fix below. - Move `detail::throwIfDetached` out of the header into a new `react/bridging/ArrayBuffer.cpp`, and drop the `AsyncArrayBuffer::throwIfDetached` wrapper that only forwarded to it. The JNI path now calls `detail::throwIfDetached` directly instead of going through `AsyncArrayBuffer`. - Add `detail::copyToOwnedBuffer`, so the C++ bridging path and the tests build an owning buffer the same way instead of each hand-rolling a `std::vector` copy. The header no longer needs `<span>`/`<string>`. - Rename the `JArrayBuffer` factories to say what they do about ownership: `createOwning`/`createOwned`/`createUnowned` become `createWithOwnedBytes`/`createWithCopiedBytes`/`createWithUnownedBytes`, matching the `RCTArrayBuffer` naming. Move `invalidate()` next to the other private members. - `RCTArrayBuffer` uses `synthesize` rather than three hand-written accessors, and normalizes `mutableBytes` to NULL for a zero-length buffer so the documented "NULL exactly when empty" invariant holds for every factory. - Run the caller's `cleanup` block before the designated initializer raises on a NULL/non-zero-length mismatch. Nothing else would ever release those bytes, so raising first leaked them. - Treat only `std::logic_error` from `tryGetMutableBuffer` as "this runtime has no native buffer" on the JNI path, and log it once, instead of swallowing every `std::exception`. - Keep the macOS mirror of `RCTArrayBuffer` byte-identical to the iOS one. Changelog: [General][Changed] - Rename the internal JArrayBuffer factories and deduplicate the ArrayBuffer helpers Differential Revision: D116358593
christophpurrer
force-pushed
the
export-D116358593
branch
from
August 19, 2026 03:26
eac2d3a to
2fcb293
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-up cleanup on the
ArrayBufferTurboModule plumbing. No behaviour change exceptthe leak fix below.
detail::throwIfDetachedout of the header into a newreact/bridging/ArrayBuffer.cpp, and drop theAsyncArrayBuffer::throwIfDetachedwrapper that only forwarded to it. The JNI path now calls
detail::throwIfDetacheddirectly instead of going through
AsyncArrayBuffer.detail::copyToOwnedBuffer, so the C++ bridging path and the tests build anowning buffer the same way instead of each hand-rolling a
std::vectorcopy. Theheader no longer needs
<span>/<string>.JArrayBufferfactories to say what they do about ownership:createOwning/createOwned/createUnownedbecomecreateWithOwnedBytes/createWithCopiedBytes/createWithUnownedBytes, matching theRCTArrayBuffernaming. Moveinvalidate()next to the other private members.RCTArrayBufferusessynthesizerather than three hand-written accessors, andnormalizes
mutableBytesto NULL for a zero-length buffer so the documented"NULL exactly when empty" invariant holds for every factory.
cleanupblock before the designated initializer raises on aNULL/non-zero-length mismatch. Nothing else would ever release those bytes, so
raising first leaked them.
std::logic_errorfromtryGetMutableBufferas "this runtime has nonative buffer" on the JNI path, and log it once, instead of swallowing every
std::exception.RCTArrayBufferbyte-identical to the iOS one.Changelog:
[General][Changed] - Rename the internal JArrayBuffer factories and deduplicate the ArrayBuffer helpers
Differential Revision: D116358593