From 33e592c755c4ec7a901b999c98d2d0542dbd713a Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Tue, 15 Sep 2026 14:47:42 -0600 Subject: [PATCH 1/5] fix: compile against V8 13 (node 24) and return the fiber's result from 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 --- fibers_async.js | 3 ++- src/fibers.cc | 34 ++++++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/fibers_async.js b/fibers_async.js index 9546a9d..c3736bc 100644 --- a/fibers_async.js +++ b/fibers_async.js @@ -11,7 +11,8 @@ function Fiber(fn, ...args) { const ar = new AsyncResource('Fiber'); const actualFn = (...args1) => ar.runInAsyncScope(() => { Fiber.current._meteor_dynamics = undefined; - fn(...args1); + // return the fiber function's value so run() resolves to it when the fiber finishes (fibers README semantics) + return fn(...args1); }); const _fiber = _Fiber(actualFn, ...args); asyncResourceWeakMap.set(_fiber, ar); diff --git a/src/fibers.cc b/src/fibers.cc index 68311a1..1ff872a 100644 --- a/src/fibers.cc +++ b/src/fibers.cc @@ -202,11 +202,13 @@ namespace uni { args.GetReturnValue().Set(handle); } template - void Return(Local handle, GetterCallbackInfo info) { + // PropertyCallbackInfo must be taken by reference: since V8 13 it holds the argument slots inline, + // so GetReturnValue().Set() on a by-value copy is silently lost. + void Return(Local handle, const GetterCallbackInfo& info) { info.GetReturnValue().Set(handle); } template - void Return(Persistent& handle, GetterCallbackInfo info) { + void Return(Persistent& handle, const GetterCallbackInfo& info) { info.GetReturnValue().Set(Local::New(Isolate::GetCurrent(), handle)); } @@ -357,7 +359,23 @@ namespace uni { } #endif -#if V8_AT_LEAST(6, 1) +#if V8_AT_LEAST(13, 0) + // FunctionCallbackInfo::Holder() was removed in V8 13; This() is the documented replacement. + inline Local Holder(const FunctionCallbackInfo& args) { return args.This(); } +#else + inline Local Holder(const FunctionCallbackInfo& args) { return args.Holder(); } +#endif + +#if V8_AT_LEAST(13, 0) + // ObjectTemplate::SetAccessor / Object::SetAccessor were removed in V8 13; SetNativeDataProperty is the replacement. + void SetAccessor( + Isolate* isolate, Local object, Local name, + FunctionType (*getter)(Local, const GetterCallbackInfo&), + void (*setter)(Local property, Local value, const SetterCallbackInfo&) = 0 + ) { + object->SetNativeDataProperty(isolate->GetCurrentContext(), name, (AccessorNameGetterCallback)getter, (AccessorNameSetterCallback)setter).ToChecked(); + } +#elif V8_AT_LEAST(6, 1) void SetAccessor( Isolate* isolate, Local object, Local name, FunctionType (*getter)(Local, const GetterCallbackInfo&), @@ -560,7 +578,7 @@ class Fiber { * be created and the callback will start. Otherwise we switch back into the exist context. */ static uni::FunctionType Run(const uni::Arguments& args) { - Fiber& that = Unwrap(args.Holder()); + Fiber& that = Unwrap(uni::Holder(args)); // There seems to be no better place to put this check.. DestroyOrphans(); @@ -601,7 +619,7 @@ class Fiber { * Throw an exception into a currently yielding fiber. */ static uni::FunctionType ThrowInto(const uni::Arguments& args) { - Fiber& that = Unwrap(args.Holder()); + Fiber& that = Unwrap(uni::Holder(args)); if (!that.yielding) { THROW(Exception::Error, "This Fiber is not yielding"); @@ -622,7 +640,7 @@ class Fiber { * effect. */ static uni::FunctionType Reset(const uni::Arguments& args) { - Fiber& that = Unwrap(args.Holder()); + Fiber& that = Unwrap(uni::Holder(args)); if (!that.started) { return uni::Return(uni::Undefined(that.isolate), args); @@ -891,7 +909,11 @@ class Fiber { uni::NewFunctionTemplate(isolate, Run, Local(), sig)); proto->Set(uni::NewLatin1Symbol(isolate, "throwInto"), uni::NewFunctionTemplate(isolate, ThrowInto, Local(), sig)); +#if V8_AT_LEAST(13, 0) + proto->SetNativeDataProperty(uni::NewLatin1Symbol(isolate, "started"), (AccessorNameGetterCallback)GetStarted); +#else proto->SetAccessor(uni::NewLatin1Symbol(isolate, "started"), GetStarted); +#endif // Global yield() function Local yield = uni::GetFunction(uni::NewFunctionTemplate(isolate, Yield_)); From 5ecb0ab9db31a9532045345ab85efb9858498a8c Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Wed, 16 Sep 2026 16:06:02 -0600 Subject: [PATCH 2/5] fix: reset the handle when a suspended fiber is garbage-collected on 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 --- src/fibers.cc | 17 ++++++++++++++++- test/orphan-gc.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/orphan-gc.js diff --git a/src/fibers.cc b/src/fibers.cc index 1ff872a..cf86a51 100644 --- a/src/fibers.cc +++ b/src/fibers.cc @@ -480,6 +480,7 @@ class Fiber { * i.e. After fiber completes, while yielded, or before started */ void MakeWeak() { + if (handle.IsEmpty()) return; // garbage-collected fiber being unwound as a zombie uni::MakeWeak(isolate, handle, (void*)this); } @@ -488,6 +489,7 @@ class Fiber { * i.e. While running. */ void ClearWeak() { + if (handle.IsEmpty()) return; // see MakeWeak() handle.ClearWeak(); } @@ -508,7 +510,14 @@ class Fiber { if (that.started) { assert(that.yielding); orphaned_fibers.push_back(&that); +#if V8_AT_LEAST(10, 4) + // kParameter is a phantom callback: V8 has already reclaimed the JS object and CHECKs that + // the handle was reset before this callback returns ("Handle not reset in first callback"). + // The fiber is unwound and deleted by DestroyOrphans; there is no JS object to hand back. + uni::Dispose(that.isolate, that.handle); +#else that.ClearWeak(); +#endif return; } @@ -550,7 +559,12 @@ class Fiber { } uni::Dispose(that.isolate, that.yielded); +#if V8_AT_LEAST(10, 4) + // The JS object is gone (see WeakCallback); nothing can reference this fiber again. + delete &that; +#else that.MakeWeak(); +#endif } } @@ -851,7 +865,8 @@ class Fiber { } static uni::FunctionType GetCurrent(Local property, const uni::GetterCallbackInfo& info) { - if (current) { + if (current && !current->handle.IsEmpty()) { + // The handle is empty while a garbage-collected fiber is being unwound as a zombie. return uni::Return(current->handle, info); } else { return uni::Return(uni::Undefined(Isolate::GetCurrent()), info); diff --git a/test/orphan-gc.js b/test/orphan-gc.js new file mode 100644 index 0000000..eacdb07 --- /dev/null +++ b/test/orphan-gc.js @@ -0,0 +1,32 @@ +"use strict"; +// A yielded fiber whose JS object is garbage-collected must be unwound as a zombie, not crash the +// process. On V8 >= 10.4 the weak callback is a phantom (kParameter) callback and has to reset its +// handle; the old kFinalizer code path resurrected the object instead. +var Fiber = require('fibers'); +var v8 = require('v8'); +var vm = require('vm'); +v8.setFlagsFromString('--expose_gc'); +var gc = vm.runInNewContext('gc'); + +var N = 200, unwound = 0; +for (var ii = 0; ii < N; ++ii) { + var fiber = Fiber(function() { + try { + Fiber.yield(); + } catch (err) { + // zombie exception. Touch Fiber.current: on V8 >= 10.4 the JS object is already gone and + // this must return undefined instead of dereferencing an empty handle. + Fiber.current; + ++unwound; + throw err; + } + }); + fiber.run(); + fiber = null; +} +gc(); gc(); +Fiber(function() {}).run(); // Fiber::Run() calls DestroyOrphans() +gc(); gc(); +Fiber(function() {}).run(); + +console.log(unwound === N ? 'pass' : 'fail: unwound ' + unwound + '/' + N); From d8ef8e2ff2e115cb5bc9d8c56925a7f3a87728a2 Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Mon, 21 Sep 2026 09:32:31 -0600 Subject: [PATCH 3/5] fix: forward Fiber.poolSize from the async wrapper to the native setter 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 --- fibers_async.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/fibers_async.js b/fibers_async.js index c3736bc..966ea58 100644 --- a/fibers_async.js +++ b/fibers_async.js @@ -28,6 +28,19 @@ Object.defineProperty(Fiber, 'current', { } }) +// poolSize is a native data property on the native constructor. On V8 >= 13 (node 24) assigning +// `Fiber.poolSize = n` on this wrapper no longer reaches that setter: it creates an own property on +// the wrapper and the native pool size silently stays at its default (120). Forward it explicitly, +// which is also what happened implicitly on node 18. +Object.defineProperty(Fiber, 'poolSize', { + get() { + return _Fiber.poolSize; + }, + set(value) { + _Fiber.poolSize = value; + }, +}) + _Fiber.prototype.runInAsyncScope = function runInAsyncScope(fn) { return asyncResourceWeakMap.get(this).runInAsyncScope(fn); From d3260f67ee556e1b5128a07fb1cf1b45131f954e Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Mon, 21 Sep 2026 09:32:31 -0600 Subject: [PATCH 4/5] fix: destroy the coroutine context before freeing its stack (CORO_PTHREAD 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 --- src/coroutine.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coroutine.cc b/src/coroutine.cc index e89abdc..a592561 100644 --- a/src/coroutine.cc +++ b/src/coroutine.cc @@ -210,13 +210,16 @@ Coroutine::Coroutine(entry_t& entry, void* arg) : } Coroutine::~Coroutine() { - if (stack.sptr) { - coro_stack_free(&stack); - } + // Destroy the context before freeing the stack: with CORO_PTHREAD the coroutine is an OS thread + // whose struct pthread glibc places at the top of the user-supplied stack, so coro_destroy's + // pthread_cancel/pthread_join must run while that memory is still mapped. #ifdef CORO_FIBER if (context.fiber) #endif (void)coro_destroy(&context); + if (stack.sptr) { + coro_stack_free(&stack); + } } Coroutine* Coroutine::create_fiber(entry_t* entry, void* arg) { From 213c42d912ad4b828bc034e8b2afdd50581d8e91 Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Mon, 21 Sep 2026 09:40:39 -0600 Subject: [PATCH 5/5] build: create bin/ recursively when installing from a source-only package afterBuild() renamed build/Release/fibers.node into bin// 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 --- build.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/build.js b/build.js index 3910fe7..2c6b212 100755 --- a/build.js +++ b/build.js @@ -95,7 +95,8 @@ function afterBuild() { var installPath = path.join(__dirname, 'bin', modPath, 'fibers.node'); try { - fs.mkdirSync(path.join(__dirname, 'bin', modPath)); + // recursive: a source-only package (no prebuilt binaries) has no bin/ directory at all + fs.mkdirSync(path.join(__dirname, 'bin', modPath), { recursive: true }); } catch (ex) {} try {