Fix two GC strong-root leaks: tail event cf cycles and abandoned pipe writes - #7017
Fix two GC strong-root leaks: tail event cf cycles and abandoned pipe writes#7017guybedford wants to merge 3 commits into
Conversation
TraceItem::FetchEventInfo::Request held Detail::cf in an untraced strong root while getCf() returns that same mutable object. A tail handler that made cf reference the request (or anything reaching its wrapper) closed an uncollectable JS<->C++ cycle: cf root -> cf -> Request wrapper -> C++ Request -> Detail -> cf root, leaking one object group per mutated tail event until isolate death. Add visitForGc to Request tracing detail->cf. Detail is shared only between the redacted/unredacted Request pair, both of this class, so every holder re-traces the handle each GC cycle. The regression test self-tails, mutates event.request.cf to reference the request, and asserts WeakRefs to both objects are reclaimed; without the fix all 16 remain alive.
The two Rc<Pipe::State> captures on the destination write promise were the only continuations in the pipe machinery not routed through ioContext.addFunctor. If the IoContext was torn down while a sink write was in flight, the awaitIo completion was destroyed without running, the Write queue entry never settled, and the bare reaction retained: write promise -> Rc<State> -> owner ref -> destination wrapper -> controller queue -> Write resolver -> write promise. Both stream wrappers plus any buffered chunks stayed on the isolate heap until isolate death, once per abandoned pipe (e.g. client disconnect mid-pipe). addFunctor's IoOwn drops the captures at IoContext teardown, matching every sibling continuation.
|
APIError: Invalid Anthropic API Key |
2 similar comments
|
APIError: Invalid Anthropic API Key |
|
APIError: Invalid Anthropic API Key |
|
@guybedford Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
Merging this PR will improve performance by 10.12%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | bm_Promise_Fib10 |
20.3 µs | 18.5 µs | +10.12% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing gbedford/gc-trace-cf-pipe-leaks (8a59ef4) with main (899b30a)
Footnotes
-
129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
2 participants
This fixes two uncollectable-retention bugs found by a systematic audit of every JS handle field that is never visited by any visitForGc (the strong-root set surfaced while testing the jsg-visit-for-gc check's blind spots).
Tail event
cfcycle.TraceItem::FetchEventInfo::Requesthad novisitForGc, holdingDetail::cfas an untraced strong root, whilegetCf()returns that same mutable object. A tail worker that makescfreference the request (or anything reaching its wrapper) closes an uncollectable JS↔C++ cycle:leaking one object group per mutated tail event until isolate death — real accumulation in long-lived tail isolates. Present since 2023. Fixed by tracing
detail->cffromRequest::visitForGc;Detailis shared only between the redacted/unredactedRequestpair, both of this class, so every holder re-traces the handle each cycle.Abandoned pipe write retention. The two
Rc<Pipe::State>captures on the destination write promise (internal.c++) were the only pipe continuations not routed throughioContext.addFunctor. If the IoContext is torn down mid-write (e.g. client disconnect during a pipe), theawaitIocompletion is destroyed without running, theWritequeue entry never settles, and the bare reaction retains: write promise →Rc<State>→ownerref → destination wrapper → controller queue →Writeresolver → write promise. Both stream wrappers plus buffered chunks persist until isolate death, once per abandoned pipe. Fixed byaddFunctor-wrapping the captures like every sibling, so the IoOwn drops them at teardown.Test coverage:
tail-cf-gc-testself-tails, mutatesevent.request.cfto reference the request, and asserts WeakRefs to both objects are reclaimed after GC — red without the fix (all 16 alive), green with it. The pipe fix's trigger (teardown racing an in-flight sink write) isn't deterministically harnessable in a wd-test, so it lands untested; the change makes the captures consistent with every neighboring continuation in the same function.