Node 24 / V8 13 compatibility with CORO_PTHREAD coroutines - #5
Open
jbaczuk-qualia wants to merge 5 commits into
Open
jbaczuk-qualia wants to merge 5 commits into
jbaczuk-qualia wants to merge 5 commits into
Conversation
…om run() - SetAccessor -> SetNativeDataProperty, Holder() -> This() (removed in V8 13) - uni::Return takes PropertyCallbackInfo by const reference: since V8 13 the argument slots live inline in the struct, so a by-value copy lost every GetReturnValue().Set() and all accessors read as undefined - fibers_async.js: return fn(...args) from runInAsyncScope so fiber.run() returns the function's result when the fiber finishes Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…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>
poolSize is a native data property on the native Fiber constructor. On V8 >= 13 (node 24) assigning `Fiber.poolSize = n` on the async wrapper no longer reaches that setter: it creates an own data property on the wrapper and the native pool size silently stays at its default of 120. On node 18 the same assignment did reach the native setter. Qualia's @qualia/patches sets Fiber.poolSize = 1e9 so coroutines are pooled forever; without this fix that became a no-op on node 24 and every finished coroutine beyond 120 concurrent ones was destroyed (an OS thread per destroy with CORO_PTHREAD, and the destroy path segfaulted, see the next commit). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…READ segfault) Coroutine::~Coroutine freed the coroutine's stack and then called coro_destroy(). With CORO_PTHREAD the coroutine is an OS thread created with pthread_attr_setstack on that stack, and glibc places the thread's `struct pthread` at the top of a user-supplied stack. coro_destroy()'s pthread_cancel(ctx->id) therefore dereferenced unmapped memory and the process died with SIGSEGV in __pthread_cancel whenever a coroutine was destroyed, i.e. whenever more than Fiber.poolSize coroutines finished. This is what has made test/pool.js and test/cleanup.js segfault on every pthread build (node 18 included); it never affected the ucontext/asm backends because nothing lives on their stacks after the coroutine stops. Destroy the context (cancel + join the thread) first, then free the stack. With this, test/pool.js, test/cleanup.js and test/orphan-gc.js pass on CORO_PTHREAD. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 21, 2026
Closed
…kage afterBuild() renamed build/Release/fibers.node into bin/<platform>/ and only created the last path component. Published tarballs always carried bin/ because they ship prebuilt binaries in it, but `npm pack` drops empty directories, so a source-only package (a vendored tarball of this branch, or a future publish without prebuilts) compiled successfully and then failed the install with ENOENT on the rename. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Author
|
GDC on the Graviton2 remote-dev host (2026-09-21, this branch + qualialabs/node#6 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes fibers build and run on node 24 (V8 13.6) while keeping the
CORO_PTHREADcoroutine backend thatbinding.gyponasync-resourcealready selects for every Linux build (and that arm64 production compiles today). Five commits on top ofasync-resource, nothing else:33e592ccompile against V8 13 and return the fiber's result fromrun()SetAccessor->SetNativeDataProperty,Holder()->This(),kFinalizer->kParameterweak callbacks;uni::Return()takesPropertyCallbackInfoby const reference (V8 13 stores the argument slots inline, so the by-value copy silently dropped every getter result);fibers_async.jsreturns the wrapped function's value fromrunInAsyncScope5ecb0abreset the handle when a suspended fiber is garbage-collected on V8 >= 10.4kParameteris a phantom callback and V8 CHECKs that the handle was reset (Handle not reset in first callback); a yielded fiber whose JS object became unreachable aborted node 20+. Same change as #4, rebasedd8ef8e2forwardFiber.poolSizefrom the async wrapper to the native setterFiber.poolSize = non the wrapper creates an own property and the native pool size silently stays 120 (on node 18 the assignment reached the native setter).@qualia/patchesrelies onFiber.poolSize = 1e9, so without this every finished coroutine beyond 120 concurrent ones was destroyedd3260f6destroy the coroutine context before freeing its stack~Coroutinefreed the stack and then calledcoro_destroy(). With pthread coroutines the thread'sstruct pthreadlives at the top of that user-supplied stack, sopthread_cancel()dereferenced unmapped memory: SIGSEGV on every coroutine destroy. This is whytest/pool.jsandtest/cleanup.jshave segfaulted on every pthread build, node 18 included213c42dcreatebin/recursively when installing from a source-only packagebuild.jsonly createdbin/<platform>/; published tarballs carrybin/because of the prebuilts,npm packof a source tree does not, sonpm cicompiled fibers and then failed the final rename with ENOENTNot in this PR, on purpose (compare #2, the
CORO_UCONTEXTport, now closed): no backend switch, no V8ThreadIdTLS key discovery, nov8_qualia_set_thread_stack_starthook. With pthread coroutines every fiber is a real OS thread, so V8's own per-thread state, itsLocker/Unlockerthread archiving and cppgc's stack scan all work as designed and need no help from fibers or from V8 patches.Node requirements
Runs on stock node 24 except for
process.binding('async_wrap'), which stock node 24 refuses:setupAsyncHacksthen disables itself and every yield corrupts the async hook stack (async hook stack has become corrupted, 7 of 19 suite failures on stock 24.21.0). qualialabs/node PR forcustom-v24-pthreadrestores the allowlist entry and fixes node 24'sthread_localTLS root cert store, which otherwise aborts on the first HTTPS request from a fiber thread. The Meteor server must run with--no-async-context-frame(Meteor'sexecutionAsyncId() === 0guard).Verification
All on arm64 Linux (M-series container), node = qualialabs/node
custom-v24-pthread(24.21.0 + 3 node-only patches, no V8 changes) unless stated.test/*.js(20 files incl.orphan-gc.js)async-resourceas shipped: 16/19 on node 18 pthread (pool.js,cleanup.js,future-exception.js)pool.js/cleanup.js/orphan-gc.jsbefored3260f6__pthread_cancel<-coro_destroy<-Coroutine::~Coroutine<-Coroutine::run(gdb,-g -O0build)d3260f6applied to fibers 5.0.4 on node 18.20 (pthread)pool.js,cleanup.jsand a 5-concurrent-coroutine destroy test pass; unpatched 5.0.4 segfaults on all threeAsyncLocalStoragecheck, timer/immediate/nextTick resumes,--stress-incremental-marking --stress-compaction--no-async-context-frame, 300 with--stress-scavenge=50, 50 withpoolSize = 4(coroutines destroyed every round)tls.createSecureContext({ca})from 3 fiber threadsCleanupQueue::Addon stock node 24)global-deployment-centerin qli5 on a Graviton2 remote-dev hostPublishing
package.jsonstill says 5.0.5, which is already published. Publish as 5.1.0 and pin it explicitly in node 24 services;^5.0.4consumers on node 18 keep 5.0.4 untouched.d8ef8e2andd3260f6also apply to node 18 and could go out as a 5.0.x later after a canary, separately from the node 24 work.🤖 Generated with Claude Code