Skip to content

Deduplicate the ArrayBuffer helpers and align the ObjC, C++ and JNI implementations (#58004) - #58004

Open
christophpurrer wants to merge 2 commits into
react:mainfrom
christophpurrer:export-D116358593
Open

Deduplicate the ArrayBuffer helpers and align the ObjC, C++ and JNI implementations (#58004)#58004
christophpurrer wants to merge 2 commits into
react:mainfrom
christophpurrer:export-D116358593

Conversation

@christophpurrer

@christophpurrer christophpurrer commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 19, 2026
@meta-codesync

meta-codesync Bot commented Aug 19, 2026

Copy link
Copy Markdown

@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
@meta-codesync meta-codesync Bot changed the title Deduplicate the ArrayBuffer helpers and align the ObjC, C++ and JNI implementations Deduplicate the ArrayBuffer helpers and align the ObjC, C++ and JNI implementations (#58004) Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant