ffi: avoid copying the threadsafe callback wrapper on the calling thread#31332
Merged
Jarred-Sumner merged 3 commits intoMay 26, 2026
Merged
Claude / Claude Code Review
completed
May 24, 2026 in 15m 6s
Code review found 1 important issue
Found 2 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/jsc/bindings/JSFFIFunction.cpp:80-81 |
ThreadSafeRefCounted wrapper missing adoptRef — debug-build assertion |
Annotations
Check failure on line 81 in src/jsc/bindings/JSFFIFunction.cpp
claude / Claude Code Review
ThreadSafeRefCounted wrapper missing adoptRef — debug-build assertion
`FFICallbackFunctionWrapper` now inherits `ThreadSafeRefCounted` but is constructed with a raw `new` and never passed through `adoptRef()` (or `relaxAdoptionRequirement()`), so on `ASSERT_ENABLED` builds the first `ref()` from `Ref { wrapper }` in `FFI_Callback_threadsafe_call` — or the `wrapper->deref()` in `FFICallbackFunctionWrapper_destroy`, which now runs for every `JSCallback.close()` — hits `ASSERT(!m_adoptionIsRequired)`. Fix: `return &adoptRef(*new FFICallbackFunctionWrapper(callbackFun
Loading