webcore: pin subprocess stdout FileReader while its pipe poll is live - #32743
Conversation
ReadableStream::from_pipe moves an already-registered pipe poll from a
subprocess PipeReader into a freshly allocated NewSource<FileReader> and
re-points the poll's owner at it. The across-read ref that keeps the
NewSource box alive (waiting_for_on_reader_done + increment_count, which
upgrades this_jsvalue to Strong) was previously only taken in
FileReader::on_start, i.e. the first time JS actually pulls from the
stream. Between from_pipe and that first pull the poll's owner pointed
into a box whose only ref was the JS wrapper's own Weak back-reference.
If the Subprocess and its cached stdout became unreachable before anyone
pulled (fire-and-forget spawn where proc.stdout is touched but never
read), GC swept the JSFileInternalReadableStreamSource wrapper and freed
the box while the poll was still armed. The next readability or EOF event
then dispatched into freed memory:
FileReader::on_reader_done FileReader.rs:1008
self.buffered.get().is_empty() heap-use-after-free
PosixBufferedReader::done PipeReader.rs:846
PosixBufferedReader::read_socket PipeReader.rs:576
file-poll dispatch <- us_internal_dispatch_ready_polls
freed by: JSDestructibleObjectDestroyFunc <- MarkedBlock sweep
Take the across-read ref in from_pipe itself, immediately after the live
reader is transferred and the JS wrapper is created, so the box is rooted
for as long as the poll can fire. on_start now checks
waiting_for_on_reader_done so it does not double-count on the same path.
The test asserts the invariant directly via heapStats rather than racing
for the crash: with the pipe's write end held open by a detached
grandchild past the direct child's exit, every
FileInternalReadableStreamSource wrapper survives GC; once the pipe EOFs,
on_reader_done releases the ref and they become collectable.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
ChangesPipe reader lifecycle
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 4:51 AM PT - Jun 26th, 2026
❌ @robobun, your commit 38df0f7 has some failures in 🧪 To try this PR locally: bunx bun-pr 32743That installs a local version of the PR into your bun-32743 --bun |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/spawn/spawn-stdout-filereader-gc-uaf.test.ts`:
- Around line 97-101: The GC/UAF regression test is too strict by asserting that
stderr is completely empty, which can be flaky under debug/ASAN. Update the test
around the spawn result handling to keep draining stderr but stop using it as
the pass/fail invariant; rely on the existing stdout and exit-code checks for
the regression signal, and only include stderr in failure diagnostics if the
test fails.
- Around line 40-43: The detached helper created in childScript can keep stdout
open forever if the fixture process crashes before the flag is written, causing
proc.stdout.text() to hang. Update the helper launched via spawn so it also
exits when the parent fixture process is gone, while still preserving the
existing flag-based completion path. Use the existing childScript setup in
spawn-stdout-filereader-gc-uaf.test.ts to add this additional termination check
so the test remains hermetic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c18a3846-75cf-49fe-9193-fc40167a6306
📒 Files selected for processing (3)
src/runtime/webcore/FileReader.rssrc/runtime/webcore/ReadableStream.rstest/js/bun/spawn/spawn-stdout-filereader-gc-uaf.test.ts
The detached sh helper now exits after ~30s even if the fixture crashes before writing the flag file, and the outer test always writes the flag in a finally block before tempDir cleanup so nothing outlives the test. stderr is still drained but only surfaced as a diagnostic when the fixture fails to produce JSON; the regression signal is the duringLivePipe / afterEof / exitCode triple.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix and test look right to me, but this is GC-lifetime/refcounting logic on the hot Bun.spawn stdout/stderr path — worth a human pass to confirm has_pending_activity() is the right gate and that there's no ref-leak path (e.g. cancel → on_reader_error with done already set).
Extended reasoning...
Overview
Two small runtime changes plus a regression test:
ReadableStream::from_pipenow takes the across-read ref (waiting_for_on_reader_done = true+increment_count(), upgradingthis_jsvaluetoStrong) immediately after transferring the live poll into the newNewSource<FileReader>, gated onreader().has_pending_activity().FileReader::on_startadds&& !self.waiting_for_on_reader_done.get()to both the POSIX and Windows non-lazy branches so the laterhandle.start()fromlazyLoadStreamdoesn't double-count.- New
spawn-stdout-filereader-gc-uaf.test.tsasserts the invariant viaheapStats().objectTypeCounts.FileInternalReadableStreamSource: ≥ N wrappers survive GC while detached grandchildren hold the write end, then ≤ 1 after EOF.
My earlier inline comment (unbounded detached helper loop) was addressed in 171024a — the loop is now capped at 1500 iterations and the outer test writes the flag in a finally before tempDir disposal.
Security risks
None in the conventional sense (no auth, no untrusted input parsing). The change is a memory-safety fix; the risk profile is getting the refcount wrong — an under-count reproduces the UAF, an over-count leaks the NewSource<FileReader> box and pins the wrapper forever. The test's afterEof <= 1 assertion guards the leak side.
Level of scrutiny
High. This is production-critical runtime code on the path of every Bun.spawn({stdout: 'pipe'}) / proc.stdout access, and it changes when a GC root is taken/released. The diff is small (~15 net runtime lines) and the reasoning in the PR description is thorough, but refcounting + GC + event-loop-poll interactions are exactly where subtle ordering bugs hide. Specific things a human reviewer should sanity-check:
has_pending_activity()is the correct gate for "poll is armed" on both POSIX and Windows afterreader().from(...)transfers the buffered reader.- The ref is always released:
on_reader_donereleases unconditionally whenwaiting_for_on_reader_done, buton_reader_erroronly releases when!done— ifon_cancel(setsdone = true) is followed by an error rather than done, the ref taken infrom_pipewould not be released. This is pre-existing behavior (same flow existed when the ref was taken inon_start), but the new code makes the ref-held window start earlier and unconditionally, so it's worth a glance. - Ordering:
to_readable_streamruns (and may throw) before the ref is taken. On the error path the poll has already been re-parented intosource— also pre-existing, but worth confirming the JS wrapper's finalizer still cleans up.
Other factors
- Bug hunter found nothing; CodeRabbit's two test-hygiene comments are resolved.
- robobun CI shows build failures on 171024a across several
build-rust/build-cppshards; they look like infra (scripts/build/ci.ts) rather than this change, but the latest green build should be confirmed before merge. - Suggested reviewers (Jarred-Sumner, dylan-conway) own this area.
|
@robobun fix: |
|
On it. The x64-asan lane shows |
…ing_activity() The x64-asan lane saw duringLivePipe=0 with the reader().has_pending_activity() gate, which resolves to FilePoll::is_active() = HasIncrementedPollCount. Switch to the same predicate FileReader::on_start already uses on this path (!reader().is_done()): after PosixBufferedReader::from() the transferred reader's flags contain none of IS_DONE / RECEIVED_EOF / CLOSED_WITHOUT_REPORTING so the ref is taken unconditionally for every live pipe, and it is released by on_reader_done / on_reader_error exactly as before. Also fold base / afterSpawn / streams into the fixture JSON and surface the whole result object in the failure diff so a repeat CI failure shows whether from_pipe ran and whether the wrappers survived the first GC pass.
|
Pushed e3358d3: switched the Verified on a local release build: the test passes 3/3 in ~130ms each. The fixture now also emits On the ref-release concern from the review: |
afterSyncGC samples the source count immediately after two synchronous Bun.gc(true) calls with no event-loop yield, so it distinguishes 'fix did not engage' (afterSyncGC < iters, wrappers swept synchronously) from 'pipes reached EOF during the sleep loop' (afterSyncGC == iters but duringLivePipe dropped afterwards). grandchildrenStarted counts touch-files written by each detached sh so a failing lane shows whether the helpers actually came up.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/webcore/ReadableStream.rs (1)
438-445: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCondense this ownership comment to the 3-line limit.
The lifetime note is useful, but this newly added comment exceeds the repo’s 3-line maximum for code comments. As per coding guidelines, “Keep code comments to 3 lines max.”
Suggested rewrite
- // The transferred reader already has a live poll registered with the - // event loop whose owner now points into this allocation. Take the - // across-read ref (and root the wrapper) immediately so a GC before - // JS first pulls cannot sweep the wrapper and free this box while the - // poll is still armed. `on_start` checks `waiting_for_on_reader_done` - // and will not take a second ref. Use the same predicate as - // `FileReader::on_start` so the ref is always paired with an eventual - // `on_reader_done`/`on_reader_error` release. + // The transferred live pipe poll points into this allocation, so hold + // the across-read ref/root before JS pulls. `on_start` sees this flag, + // avoiding a duplicate ref; reader-done/error releases it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/webcore/ReadableStream.rs` around lines 438 - 445, Condense the ownership note in ReadableStream transfer handling to fit the 3-line comment limit while preserving the key lifetime guarantee. Keep the guidance near the existing allocation/poll logic and mention the shared predicate with FileReader::on_start plus the paired on_reader_done/on_reader_error release, but remove the extra explanatory detail so the comment stays brief and actionable.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/runtime/webcore/ReadableStream.rs`:
- Around line 438-445: Condense the ownership note in ReadableStream transfer
handling to fit the 3-line comment limit while preserving the key lifetime
guarantee. Keep the guidance near the existing allocation/poll logic and mention
the shared predicate with FileReader::on_start plus the paired
on_reader_done/on_reader_error release, but remove the extra explanatory detail
so the comment stays brief and actionable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4979a52a-0b2b-4bde-a52d-dc42f3cc8630
📒 Files selected for processing (2)
src/runtime/webcore/ReadableStream.rstest/js/bun/spawn/spawn-stdout-filereader-gc-uaf.test.ts
Replace the sh flag-polling loop with 'exec sleep 30' so the grandchild cannot exit early if fractional sleep or the flag path behave differently on a CI runner, and have the direct child linger 10ms after spawning it so the grandchild definitely holds fd 1 before the child closes its copy. The fixture reads each grandchild pid from the started/ touch-files and SIGTERMs them to drive EOF; the outer test SIGKILLs any survivors in its finally. Also condense the from_pipe ownership comment to three lines.
The CI runner sets BUN_FEATURE_FLAG_NO_ORPHANS=1 on ASAN lanes only, which arms PR_SET_CHILD_SUBREAPER around each spawn and kills subreaper-adopted daemons as soon as their intermediate parent exits. That is exactly the process shape this test depends on: a detached grandchild that inherits the direct child's stdout and holds the write end open past the child's exit. With the flag set, the grandchildren were killed the moment the direct children exited, the pipes EOF'd, and on_reader_done released the across-read ref before the fixture could observe it, which is why only the x64-asan lane saw duringLivePipe=0 while debug/release/release-asan all passed locally. The test already enforces the invariant NO_ORPHANS exists for: the fixture SIGTERMs every grandchild by pid and the outer test SIGKILLs survivors in a finally block, so nothing can outlive it.
|
Root-caused the x64-asan-only failure: the CI runner sets So on the asan lane the grandchildren were killed as soon as each direct child exited, every pipe EOF'd, and The fix is in the test (d7c5b0b): clear The runtime fix itself ( |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/spawn/spawn-stdout-filereader-gc-uaf.test.ts`:
- Around line 49-51: The test currently relies on a fixed 10ms delay in the
intermediate shell setup, which does not guarantee the detached child has
created its pid file before the directory snapshot is taken. Update the spawn
test flow around the setup used by the `spawn`/`setTimeout(..., 10)` step and
the later pid-file snapshot logic to wait for the actual pid file condition
instead of sleeping, so the test proceeds only after the detached shell has
written its pid file and stdout can close deterministically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48ea0918-7dd4-476e-9226-05c63732281f
📒 Files selected for processing (2)
src/runtime/webcore/ReadableStream.rstest/js/bun/spawn/spawn-stdout-filereader-gc-uaf.test.ts
Replace the 10ms setTimeout with a bounded poll for the grandchild's pid file, so the direct child only exits once the grandchild is provably past posix_spawn (owning fd 1) and its pid is recorded. await proc.exited then implies the fixture's readdirSync sees every grandchild, making the kill loop deterministic instead of racing a timer.
|
CI update: on the latest push (26383ab) the test passes on the x64-asan lane that had been failing every build ( The only hard-failed job in that build is If the darwin artifact download flakes again, the diff is ready: the runtime change is small (root the |
|
This is ready for review; the remaining CI red is infrastructure, not the diff. Across the last three builds (64926, 64933, and the The Everything that actually exercises the change is green:
All review threads (claude and CodeRabbit) are addressed and resolved. |
… and on_reader_error (#32921) ### What `Bun.spawn` stdout/stderr pipe readers still hit a heap-use-after-free on current main under fault injection, with the faulting pc symbolizing to `PosixBufferedReader::on_error` and `PosixBufferedReader::register_poll`. #32743 fixed the read-completion path of this class (`on_reader_done` reached from `read_socket`) but the error and poll-registration completion paths have the same lifetime defect. ### Cause `PosixBufferedReader::read_with_fn` holds `&mut` into the `NewSource<FileReader>` box for the whole epoll dispatch. Mid-loop, `FileReader::on_read_chunk` resolves the pending `read()` promise via `p.run()`, which enters JS and drains microtasks. If user JS calls `reader.cancel()` there, the chain ``` cancel -> on_cancel -> reader.close() -> done() -> on_reader_done() ``` runs synchronously and releases the across-read ref that #32743 added. That drops the box's count to the JS wrapper's own ref and downgrades the wrapper from Strong to Weak. A GC inside that same microtask drain sweeps the wrapper, and its finalizer frees the box out from under `read_with_fn`. On the next inner-loop iteration the (now closed) fd returns `EBADF` and read_with_fn calls `parent.on_error(err)` on the freed reader, or `parent.register_poll()` on the retry arm, reproducing the reported frames. `FileReader::on_reader_error` has the identical shape via its own `p.run()` followed by reads of `self`. Both callbacks are reachable from `Bun.spawn` and from `Bun.file(fd).stream()`, so the blast radius is any app that streams a subprocess pipe. ### Fix Hold one additional ref on the `NewSource` box across `p.run()` at both call sites (`on_read_chunk` and `on_reader_error`), released at true tail. The re-entrant release from `on_reader_done` then lands at a count of two instead of one, so it never crosses the downgrade threshold and the wrapper stays Strong-rooted for the remainder of the dispatch. The box is collected normally on a later, off-stack GC. `on_read_chunk` also now returns `false` when the re-entrant cancel marked the reader done. `read_with_fn`'s two flush sites consult that return only when `received_hup` is false, so this stops the non-HUP path from issuing another `recv` on the cancelled reader's closed fd (the source of the spurious `EBADF` that previously reached `on_error`). The HUP-path flushes intentionally ignore it (`&& !received_hup` is a documented hang fix for shell blocking pipes), so they still loop to EOF; that is pre-existing, benign, and left alone, because the pin and not the return value is what makes both branches memory safe, and tightening it means changing `PipeReader.rs` infrastructure shared by every buffered-reader parent. The sibling Posix site at `PipeReader::start` (SubprocessPipeReader.rs) already holds a `ScopedRef` across the same re-entrancy and is unaffected. ### Verification `test/js/bun/spawn/spawn-stdout-filereader-gc-uaf.test.ts` gains a second test next to the #32743 one. A detached grandchild saturates the stdout socketpair while the parent is blocked in `sleepSync`, so the payload arrives in one poll dispatch and `p.run()` fires with `read_with_fn` still on the stack. Which of `read_with_fn`'s two flush sites delivers it depends on the kernel socketpair buffer (the 128 KiB mid-loop flush on Linux, the retry flush on macOS, whose default is a few KiB); the pin guards both identically, so the test does not depend on a platform-specific buffer size. The `.then` callback cancels the reader and samples `heapStats().protectedObjectTypeCounts.FileInternalReadableStreamSource` immediately after. That counter observes the `JsRef` Strong directly, so the test asserts the lifetime invariant rather than racing a GC into the vulnerable window, and is deterministic in both directions (no ASan or `collectContinuously` dependence): - without the `src/` change: `protectedAfter` is `0` (Strong dropped mid-dispatch, the UAF precondition) and the test fails on that assertion with both preconditions green - with it: `protectedAfter` is `1` and both tests in the file pass The `on_reader_error` pin is the same bracket at the sibling site named by the reported `register_poll` frame, whose failure path calls it. It is not separately tested, and that is a proven limit rather than an untried one. Through the only live `Pending` consumer (the pull promise), the window cannot be reached from JS: the native pull promise's rejection arm in `#pull` errors the stream before any user reaction runs, and `readableStreamCancel` is a no-op on an errored stream, so a `reader.cancel()` inside a rejection handler never re-enters `on_reader_done`. I verified this empirically with the same dup2 fd swap `spawn-pipe-read-error-leak.test.ts` uses to force a non-retry `recv` error: `on_reader_error` fires, the in-handler `cancel()` produces no `onReaderDone` and no change in the protected count on either build, so no assertion through that route distinguishes fixed from unfixed. The `PendingFuture::Handler` consumer, the only other route to `p.run()` there, has no installers. The pin stays because `on_reader_error` reads `self` after a call that runs user JS; that contract holds today only through the non-local ordering inside `#pull`'s JS, which nothing enforces, and the bracket is balanced and free. Also re-ran `spawn-pipe-read-error-leak`, `spawn-streaming-stdout`, `spawn-unread-stdout-gc`, `spawn-stdout-iterate-leak`, `readablestream-helpers`, `spawn-ipc-gc`, and `native-source-onclose-leak`: all green.
### What `FileReader::on_reader_done` has the same shape #32921 fixed in its two siblings: it runs user JavaScript and then reads `self` with no refcount pin. A heap-use-after-free was reported there under fault injection on an instrumented build (READ in `FileReader::on_reader_done`, on the `self.buffered` length read that follows `p.run()`). #32921 added the pin to `on_read_chunk` and `on_reader_error` but not to `on_reader_done`, one screen below them in the same file. ### Cause `on_reader_done` resolves the pending pull with `Done`/`OwnedAndDone` via `p.run()`, which fulfills a JS promise and drains microtasks. After that returns, it still reads `self.buffered`, calls `(*self.parent()).on_close()` (which can invoke a native consumer's `close_handler`), and reads `self.waiting_for_on_reader_done`. `self` is a field of a heap-allocated, refcounted `NewSource<FileReader>`; nothing holds an extra reference across those calls, so any release that lands inside them frees the box out from under the rest of the function. On current `main` the function is safe only through a non-local invariant: while `p.run()` is executing, the count is 2 (the JS wrapper's ref plus the across-read ref that `on_reader_done` itself releases at its tail), and no synchronous release is reachable from JS inside that window, because the re-entrant `cancel -> on_cancel -> reader().close() -> done()` chain that #32921's test drives is gated on `!self.reader().is_done()`, which is always false once `on_reader_done` is on the stack. That is the same situation #32921 described for its `on_reader_error` pin: the contract holds today only through non-local ordering that nothing enforces. ### Fix Hold one additional ref on the `NewSource` box for the duration of `on_reader_done`, released at true tail, matching the bracket `on_read_chunk` and `on_reader_error` already have. The across-read release then lands at a count of two instead of one, the wrapper stays Strong-rooted for the rest of the dispatch, and the box is collected normally on a later, off-stack GC. I audited the other `Pending::run` call sites in `src/runtime/webcore`. `ByteStream::on_data`'s is tail-positioned, `ByteStream::on_cancel`'s is entered from JS with the wrapper a conservative stack root, and `ByteStream::finalize` defers any JS-visible resolution to the next tick. `on_reader_done` was the only remaining run-JS-then-read-`self` site without a pin. ### Verification `test/js/bun/spawn/spawn-stdout-filereader-gc-uaf.test.ts` (the file holding the #32743 and #32921 tests) gains a third test for the third member of the family. `cat` holds the pipe open until its stdin EOFs, so `reader.read()` is armed before the pipe can close; `stdin.end()` then drives `on_reader_done` with the pull Pending and no fixed sleeps. The `{done: true}` handler, which runs synchronously inside `on_reader_done`'s `p.run()` microtask drain, asserts the source stays Strong-protected through the most aggressive teardown reachable from there (`cancel()` plus dropping every JS reference plus `Bun.gc(true)`). Being straight about what that test is: it passes on the unfixed build too, because the across-read ref carries the invariant today (see Cause). It is a guard on the invariant the pin formalizes, and the canary for the day a change makes the release reachable from inside `p.run()`, which is exactly what happened to `on_read_chunk` between #32743 and #32921. I could not construct a plain-JS input that distinguishes the pin. In a 215-iteration probe on the unfixed ASan debug build across five shapes (pipe EOF with a pending read plus an in-handler `cancel()`, the chunk-then-done variant, `node:child_process` `end` and `close` handlers, and a cancel-initiated `on_reader_done`), with `Malloc=1` and `BUN_JSC_collectContinuously=1`, ASan never fired and the `FileInternalReadableStreamSource` count never dropped inside the window. Also re-ran `spawn-streaming-stdout`, `spawn-unread-stdout-gc`, `spawn-stdout-iterate-leak`, `spawn-pipe-read-error-leak`, `spawn-ipc-gc`, and `native-source-onclose-leak` on the debug (ASan plus debug_assert) build: all green, which also checks the bracket's refcount balance on every path those exercise.
2 participants
What
ReadableStream::from_pipe(theproc.stdout/proc.stderrpath forBun.spawnand the shell subprocess) moves an already-registered pipe poll from the subprocessPipeReaderinto a freshly allocatedNewSource<FileReader>and re-points the poll's owner at it. The across-read ref that keeps that box alive (waiting_for_on_reader_done+increment_count(), which upgradesthis_jsvaluetoStrong) was only taken inFileReader::on_start, i.e. the first time JS actually pulls from the stream.Between
from_pipeand that first pull, the poll's owner points into a box whose only ref is the JS wrapper's ownWeakback-reference. If theSubprocessand its cached stdout become unreachable before anyone pulls (a fire-and-forget spawn whereproc.stdoutis touched but never read, and the direct child exits while something else still holds the write end), GC sweeps theJSFileInternalReadableStreamSourcewrapper and frees theNewSource<FileReader>box while the poll is still armed. The next readability or EOF event dispatches into freed memory:Found by a coverage-guided GC-stress fuzzer with syscall interposition (
BUN_JSC_collectContinuously=1plus an injectedEAGAINto keep the read pending). In release builds this is silent heap corruption.Fix
Take the across-read ref in
from_pipeitself, immediately after the live reader is transferred and the JS wrapper is created, so the box isStrong-rooted for as long as the poll can fire.on_reader_done/on_reader_errorrelease it exactly as before.FileReader::on_startnow checkswaiting_for_on_reader_donebefore taking the ref so the laterhandle.start()call fromlazyLoadStreamdoes not double-count on this path.How did you verify your code works?
The test asserts the lifetime invariant directly via
heapStats().objectTypeCounts.FileInternalReadableStreamSourcerather than racing for the crash, since the exact UAF trigger depends on the fuzzer's syscall interposition. A detached grandchild (sh -c 'while [ ! -e FLAG ]; do sleep 0.02; done; echo x') inherits the child's stdout and keeps the write end open past the direct child's exit, so theFileReader's poll is still armed while we force GC with nothing in JS referencing the wrapper.git stash push -- src/+bun bd test):duringLivePipe = 0of 4; every wrapper swept while its poll owner still points into the freed box.duringLivePipe >= 4; once the grandchildren exit and the pipes EOF,afterEof <= 1(one may remain via a conservatively-rooted finalSubprocess, same caveat asspawn-ipc-gc.test.ts).Also passes
spawn-streaming-stdout.test.ts,spawn-unread-stdout-gc.test.ts,spawn-ipc-gc.test.ts,spawn-stdout-iterate-leak.test.ts, andreadablestream-helpers.test.ts.