Skip to content

fix: reset the handle when a suspended fiber is garbage-collected on V8 >= 10.4 - #4

Closed
jbaczuk-qualia wants to merge 1 commit into
node24-compatfrom
fix/orphan-fiber-phantom-weak-callback
Closed

jbaczuk-qualia wants to merge 1 commit into
node24-compatfrom
fix/orphan-fiber-phantom-weak-callback

Conversation

@jbaczuk-qualia

Copy link
Copy Markdown

Problem

f3cbba6 ("Make compatible with v20", PR #1) registers the Fiber weak callback with WeakCallbackType::kParameter because V8 removed kFinalizer. The two have different contracts:

  • kFinalizer (node 18, V8 10.2): the callback runs before the object is reclaimed and may resurrect it. Fiber::WeakCallback relied on this for suspended fibers: ClearWeak(), unwind later in DestroyOrphans(), MakeWeak() again.
  • kParameter (node 20+): a phantom callback. The object is already gone and V8 CHECKs that the callback reset its handle (global-handles.cc: "Handle not reset in first callback").

So on node 20+ a yielded fiber whose JS object becomes unreachable (its resuming callback was dropped: an abandoned request, a cleared timer, a socket closed without firing) aborts the process at the next GC:

# Fatal error in , line 0
# Check failed: Handle not reset in first callback. See comments on |v8::WeakCallbackInfo|.

Finished and never-started fibers were fine (delete → destructor resets the handle), which is why the suite and the GDC click-throughs never hit it. Node 18 is unaffected.

Fix

On V8_AT_LEAST(10, 4):

  • WeakCallback: for a started (yielded) fiber, queue it as an orphan and reset the handle instead of ClearWeak().
  • DestroyOrphans: after UnwindStack(), delete the fiber instead of MakeWeak() on a handle that no longer exists.
  • MakeWeak(), ClearWeak() and the Fiber.current getter return early on an empty handle. Fiber::Yield_ and user code in the zombie's catch/finally reach these while the stack unwinds; the first cut without the guards segfaulted in GlobalHandles::ClearWeakness.

The node 18 code path is unchanged.

Test

test/orphan-gc.js: 200 yielded fibers dropped, two forced GCs, a Fiber.run() to trigger DestroyOrphans, and a check that all 200 unwound (each touches Fiber.current from its catch).

build result
node24-compat unpatched, node 24.21.0 (patched custom-v24, ucontext) abort, Check failed: Handle not reset in first callback
this branch, node 24.21.0 pass; full suite 20/20
fibers 5.0.4 on node 18.16.1 (kFinalizer path) pass

Notes

🤖 Generated with Claude Code

…V8 >= 10.4

f3cbba6 ("Make compatible with v20") registers the Fiber weak callback with
WeakCallbackType::kParameter because V8 removed kFinalizer. The two types have
different contracts: kFinalizer ran before the object was reclaimed and let the
callback resurrect it, which Fiber::WeakCallback relied on for suspended fibers
(ClearWeak(), unwind later in DestroyOrphans, MakeWeak() again). kParameter is a
phantom callback: the object is already gone and V8 CHECKs that the callback reset
its handle ("Handle not reset in first callback", global-handles.cc), so a yielded
fiber whose JS object becomes unreachable aborted the process on node 20+:

    # Fatal error in , line 0
    # Check failed: Handle not reset in first callback. See comments on |v8::WeakCallbackInfo|.

On V8 >= 10.4 the orphan branch now resets the handle in the callback and
DestroyOrphans deletes the fiber after unwinding it instead of re-weakening a handle
that no longer exists. MakeWeak(), ClearWeak() and the Fiber.current getter tolerate
the empty handle, which Fiber::Yield_ and JS code in the zombie's catch/finally
blocks hit while the stack unwinds (the first version without the guards segfaulted
in GlobalHandles::ClearWeakness). The node 18 (V8 10.2) code path is unchanged.

test/orphan-gc.js garbage-collects 200 yielded fibers, forces DestroyOrphans and
checks every fiber was unwound; it aborts on the unpatched build and passes here on
node 24.21.0 (patched custom-v24, ucontext) together with the other 19 tests, and
passes on node 18.16.1 with fibers 5.0.4.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jbaczuk-qualia

Copy link
Copy Markdown
Author

Closed 2026-09-21: carried into #5 as 5ecb0ab (rebased onto async-resource without the ucontext commits). This fix is backend-independent; it was based on node24-compat, the CORO_UCONTEXT branch that we are not shipping. With pthread coroutines the same test (test/orphan-gc.js) also needed the destructor-order fix in #5 (d3260f6): unwinding orphans beyond the pool size destroyed coroutines, and coro_destroy segfaulted on pthread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant