Skip to content

Run the With hooks around WithRuntimeDecorator::createArrayBuffer (#58935) - #58935

Closed
lavenzg wants to merge 1 commit into
react:mainfrom
lavenzg:export-D123939407
Closed

lavenzg wants to merge 1 commit into
react:mainfrom
lavenzg:export-D123939407

Conversation

@lavenzg

@lavenzg lavenzg commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary:

WithRuntimeDecorator wraps every override in Around around{with_}; except createArrayBuffer(std::shared_ptr<MutableBuffer>), which calls RD::createArrayBuffer directly. It has been like that since external ArrayBuffers were added in facebook/hermes#793, where the overrides next to it did get the guard. HermesRuntimeImpl::createArrayBuffer allocates a JSArrayBuffer on the GC heap, so on makeThreadSafeHermesRuntime it runs without the runtime lock, and reentrancy checks built on WithRuntimeDecorator never see the call.

#45042 and #45049 fixed the same kind of gap for methods that had no WithRuntimeDecorator override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the makeA() calls can not be interleaved assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of decorator.h has the same line.

X-link: facebook/hermes#2204

Test Plan:
Added HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith. It wraps makeHermesRuntime() in a counting WithRuntimeDecorator, creates an ArrayBuffer from a MutableBuffer, and checks that before() and after() each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full APITests binary (356 tests) in a Debug + ASan build.

🤖 Generated with Claude Code

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg

@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 Oct 7, 2026
@facebook-github-tools facebook-github-tools Bot added p: Facebook Partner: Facebook Partner labels Oct 7, 2026
@meta-codesync

meta-codesync Bot commented Oct 7, 2026

Copy link
Copy Markdown

@lavenzg has exported this pull request. If you are a Meta employee, you can view the originating Diff in D123939407.

@meta-codesync meta-codesync Bot changed the title Run the With hooks around WithRuntimeDecorator::createArrayBuffer Run the With hooks around WithRuntimeDecorator::createArrayBuffer (#58935) Oct 7, 2026
lavenzg pushed a commit to lavenzg/react-native that referenced this pull request Oct 7, 2026
…act#58935)

Summary:


`WithRuntimeDecorator` wraps every override in `Around around{with_};` except `createArrayBuffer(std::shared_ptr<MutableBuffer>)`, which calls `RD::createArrayBuffer` directly. It has been like that since external ArrayBuffers were added in facebook/hermes#793, where the overrides next to it did get the guard. `HermesRuntimeImpl::createArrayBuffer` allocates a `JSArrayBuffer` on the GC heap, so on `makeThreadSafeHermesRuntime` it runs without the runtime lock, and reentrancy checks built on `WithRuntimeDecorator` never see the call.

react#45042 and react#45049 fixed the same kind of gap for methods that had no `WithRuntimeDecorator` override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the `makeA() calls can not be interleaved` assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of `decorator.h` has the same line.

X-link: facebook/hermes#2204

Test Plan:
Added `HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith`. It wraps `makeHermesRuntime()` in a counting `WithRuntimeDecorator`, creates an ArrayBuffer from a `MutableBuffer`, and checks that `before()` and `after()` each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full `APITests` binary (356 tests) in a Debug + ASan build.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg
@lavenzg
lavenzg force-pushed the export-D123939407 branch from 4986e6d to 6de1a6e Compare October 7, 2026 21:05
lavenzg pushed a commit to lavenzg/react-native that referenced this pull request Oct 7, 2026
…act#58935)

Summary:
Pull Request resolved: react#58935

`WithRuntimeDecorator` wraps every override in `Around around{with_};` except `createArrayBuffer(std::shared_ptr<MutableBuffer>)`, which calls `RD::createArrayBuffer` directly. It has been like that since external ArrayBuffers were added in facebook/hermes#793, where the overrides next to it did get the guard. `HermesRuntimeImpl::createArrayBuffer` allocates a `JSArrayBuffer` on the GC heap, so on `makeThreadSafeHermesRuntime` it runs without the runtime lock, and reentrancy checks built on `WithRuntimeDecorator` never see the call.

react#45042 and react#45049 fixed the same kind of gap for methods that had no `WithRuntimeDecorator` override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the `makeA() calls can not be interleaved` assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of `decorator.h` has the same line.

X-link: facebook/hermes#2204

Test Plan:
Added `HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith`. It wraps `makeHermesRuntime()` in a counting `WithRuntimeDecorator`, creates an ArrayBuffer from a `MutableBuffer`, and checks that `before()` and `after()` each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full `APITests` binary (356 tests) in a Debug + ASan build.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg
@lavenzg
lavenzg force-pushed the export-D123939407 branch 2 times, most recently from 990ad31 to 6fc7b2d Compare October 8, 2026 16:50
lavenzg pushed a commit to lavenzg/react-native that referenced this pull request Oct 8, 2026
…act#58935)

Summary:


`WithRuntimeDecorator` wraps every override in `Around around{with_};` except `createArrayBuffer(std::shared_ptr<MutableBuffer>)`, which calls `RD::createArrayBuffer` directly. It has been like that since external ArrayBuffers were added in facebook/hermes#793, where the overrides next to it did get the guard. `HermesRuntimeImpl::createArrayBuffer` allocates a `JSArrayBuffer` on the GC heap, so on `makeThreadSafeHermesRuntime` it runs without the runtime lock, and reentrancy checks built on `WithRuntimeDecorator` never see the call.

react#45042 and react#45049 fixed the same kind of gap for methods that had no `WithRuntimeDecorator` override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the `makeA() calls can not be interleaved` assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of `decorator.h` has the same line.

X-link: facebook/hermes#2204

Test Plan:
Added `HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith`. It wraps `makeHermesRuntime()` in a counting `WithRuntimeDecorator`, creates an ArrayBuffer from a `MutableBuffer`, and checks that `before()` and `after()` each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full `APITests` binary (356 tests) in a Debug + ASan build.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg
…act#58935)

Summary:
Pull Request resolved: react#58935

`WithRuntimeDecorator` wraps every override in `Around around{with_};` except `createArrayBuffer(std::shared_ptr<MutableBuffer>)`, which calls `RD::createArrayBuffer` directly. It has been like that since external ArrayBuffers were added in facebook/hermes#793, where the overrides next to it did get the guard. `HermesRuntimeImpl::createArrayBuffer` allocates a `JSArrayBuffer` on the GC heap, so on `makeThreadSafeHermesRuntime` it runs without the runtime lock, and reentrancy checks built on `WithRuntimeDecorator` never see the call.

react#45042 and react#45049 fixed the same kind of gap for methods that had no `WithRuntimeDecorator` override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the `makeA() calls can not be interleaved` assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of `decorator.h` has the same line.

X-link: facebook/hermes#2204

Test Plan:
Added `HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith`. It wraps `makeHermesRuntime()` in a counting `WithRuntimeDecorator`, creates an ArrayBuffer from a `MutableBuffer`, and checks that `before()` and `after()` each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full `APITests` binary (356 tests) in a Debug + ASan build.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg
@lavenzg
lavenzg force-pushed the export-D123939407 branch from 6fc7b2d to 1e60a59 Compare October 8, 2026 16:53
meta-codesync Bot pushed a commit to facebook/hermes that referenced this pull request Oct 8, 2026
)

Summary:
X-link: react/react-native#58935

`WithRuntimeDecorator` wraps every override in `Around around{with_};` except `createArrayBuffer(std::shared_ptr<MutableBuffer>)`, which calls `RD::createArrayBuffer` directly. It has been like that since external ArrayBuffers were added in #793, where the overrides next to it did get the guard. `HermesRuntimeImpl::createArrayBuffer` allocates a `JSArrayBuffer` on the GC heap, so on `makeThreadSafeHermesRuntime` it runs without the runtime lock, and reentrancy checks built on `WithRuntimeDecorator` never see the call.

react/react-native#45042 and react/react-native#45049 fixed the same kind of gap for methods that had no `WithRuntimeDecorator` override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the `makeA() calls can not be interleaved` assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of `decorator.h` has the same line.

Pull Request resolved: #2204

Test Plan:
Added `HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith`. It wraps `makeHermesRuntime()` in a counting `WithRuntimeDecorator`, creates an ArrayBuffer from a `MutableBuffer`, and checks that `before()` and `after()` each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full `APITests` binary (356 tests) in a Debug + ASan build.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg

fbshipit-source-id: 165a7807ecc9cf68f56a1f2b188642dbd68d136d
meta-codesync Bot pushed a commit to facebook/hermes that referenced this pull request Oct 8, 2026
)

Summary:
X-link: react/react-native#58935

`WithRuntimeDecorator` wraps every override in `Around around{with_};` except `createArrayBuffer(std::shared_ptr<MutableBuffer>)`, which calls `RD::createArrayBuffer` directly. It has been like that since external ArrayBuffers were added in #793, where the overrides next to it did get the guard. `HermesRuntimeImpl::createArrayBuffer` allocates a `JSArrayBuffer` on the GC heap, so on `makeThreadSafeHermesRuntime` it runs without the runtime lock, and reentrancy checks built on `WithRuntimeDecorator` never see the call.

react/react-native#45042 and react/react-native#45049 fixed the same kind of gap for methods that had no `WithRuntimeDecorator` override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the `makeA() calls can not be interleaved` assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of `decorator.h` has the same line.

Pull Request resolved: #2204

Test Plan:
Added `HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith`. It wraps `makeHermesRuntime()` in a counting `WithRuntimeDecorator`, creates an ArrayBuffer from a `MutableBuffer`, and checks that `before()` and `after()` each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full `APITests` binary (356 tests) in a Debug + ASan build.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg

fbshipit-source-id: 165a7807ecc9cf68f56a1f2b188642dbd68d136d
@meta-codesync meta-codesync Bot closed this in 82def6e Oct 8, 2026
@meta-codesync meta-codesync Bot added the Merged This PR has been merged. label Oct 8, 2026
@meta-codesync

meta-codesync Bot commented Oct 8, 2026

Copy link
Copy Markdown

@lavenzg merged this pull request in 82def6e.

meta-codesync Bot pushed a commit to facebook/hermes that referenced this pull request Oct 8, 2026
)

Summary:
X-link: react/react-native#58935

`WithRuntimeDecorator` wraps every override in `Around around{with_};` except `createArrayBuffer(std::shared_ptr<MutableBuffer>)`, which calls `RD::createArrayBuffer` directly. It has been like that since external ArrayBuffers were added in #793, where the overrides next to it did get the guard. `HermesRuntimeImpl::createArrayBuffer` allocates a `JSArrayBuffer` on the GC heap, so on `makeThreadSafeHermesRuntime` it runs without the runtime lock, and reentrancy checks built on `WithRuntimeDecorator` never see the call.

react/react-native#45042 and react/react-native#45049 fixed the same kind of gap for methods that had no `WithRuntimeDecorator` override at all. This one has an override that just skips the guard, so it wasn't caught there.

A small stress program shows the race: one thread runs an allocating JS loop on a thread-safe runtime while another creates external ArrayBuffers. In a Debug build it fails the `makeA() calls can not be interleaved` assert in HadesGC. With the guard it runs clean. I didn't add it as a test because it depends on timing. React Native's copy of `decorator.h` has the same line.

Pull Request resolved: #2204

Test Plan:
Added `HermesRuntimeDecoratorTest.CreateArrayBufferCallsWith`. It wraps `makeHermesRuntime()` in a counting `WithRuntimeDecorator`, creates an ArrayBuffer from a `MutableBuffer`, and checks that `before()` and `after()` each ran once. Without the change both counts are 0 and the test fails. With it the test passes, and so does the full `APITests` binary (356 tests) in a Debug + ASan build.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed By: avp

Differential Revision: D123939407

Pulled By: lavenzg

fbshipit-source-id: 165a7807ecc9cf68f56a1f2b188642dbd68d136d
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. Merged This PR has been merged. meta-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants