From e0fbe775882bd3505b0a6f60eb45598fb93ba42d Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Tue, 15 Sep 2026 14:47:42 -0600 Subject: [PATCH 1/4] 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 a7f575069509a1b17d34ec5eea10d0ed7c70e9a6 Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Tue, 15 Sep 2026 14:47:42 -0600 Subject: [PATCH 2/4] build: use ucontext coroutines on Linux and arm64 instead of pthreads V8 13 keeps GC state such as the marking write barrier in compiler thread_local storage that is not copied to coroutine threads, so the CORO_PTHREAD build corrupts the heap under node 24. Same-thread coroutines (upstream's default, and the async-resource-threadless branch) see all of it. Co-Authored-By: Claude Fable 5.1 --- binding.gyp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/binding.gyp b/binding.gyp index 824ce58..15e5969 100644 --- a/binding.gyp +++ b/binding.gyp @@ -41,7 +41,15 @@ ['OS == "linux"', { 'cflags_c': [ '-std=gnu11' ], - 'defines': ['CORO_PTHREAD'], + 'variables': { + 'USE_MUSL': '&1 | head -n1 | grep "musl" | wc -l)', + }, + 'conditions': [ + ['<(USE_MUSL) == 1', + {'defines': ['CORO_ASM', '__MUSL__']}, + {'defines': ['CORO_UCONTEXT']} + ], + ], }, ], ['OS == "solaris" or OS == "sunos" or OS == "freebsd" or OS == "aix"', {'defines': ['CORO_UCONTEXT']}], @@ -50,15 +58,15 @@ ['target_arch == "arm"', { # There's been problems getting real fibers working on arm - 'defines': ['CORO_PTHREAD'], - 'defines!': ['CORO_UCONTEXT', 'CORO_SJLJ', 'CORO_ASM'], + 'defines': ['CORO_UCONTEXT', '_XOPEN_SOURCE'], + 'defines!': ['CORO_PTHREAD', 'CORO_SJLJ', 'CORO_ASM'], }, ], ['target_arch == "arm64"', { # There's been problems getting real fibers working on arm - 'defines': ['CORO_PTHREAD'], - 'defines!': ['CORO_UCONTEXT', 'CORO_SJLJ', 'CORO_ASM'], + 'defines': ['CORO_UCONTEXT', '_XOPEN_SOURCE'], + 'defines!': ['CORO_PTHREAD', 'CORO_SJLJ', 'CORO_ASM'], }, ], ], From a692f794f9d4e94f223940c310249d8ddc14d5b4 Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Tue, 15 Sep 2026 14:47:42 -0600 Subject: [PATCH 3/4] fix: find V8's ThreadId TLS key on V8 >= 12 and report the running stack to V8 find_thread_id_key located V8's isolate, thread-data and thread-id pthread keys by value. V8 >= 12 keeps the first two in thread_local storage, so the scan silently found nothing (its asserts are compiled out) and coroutines stopped getting their own V8 ThreadId, which the Locker/Unlocker archiving of JS stacks relies on. Fall back to comparing two helper-thread snapshots: the ThreadId slot is the small int that increments between them. The isolate and thread-data slots are maintained by Locker/Unlocker themselves and are only swapped when found. FIBERS_DEBUG_TLS=1 prints what was detected. If the node binary exports v8_qualia_set_thread_stack_start (Qualia's node build), call it on every coroutine switch with the target stack's start so cppgc's conservative stack scan, stack limits and IsOnStack() see the coroutine stack instead of the OS thread's. Co-Authored-By: Claude Fable 5.1 --- src/coroutine.cc | 188 +++++++++++++++++++++++++++++++++++------------ src/coroutine.h | 5 ++ 2 files changed, 145 insertions(+), 48 deletions(-) diff --git a/src/coroutine.cc b/src/coroutine.cc index e89abdc..dd69b6a 100644 --- a/src/coroutine.cc +++ b/src/coroutine.cc @@ -17,6 +17,11 @@ #endif #include +#include +#include +#ifndef WINDOWS +#include +#endif #include #include using namespace std; @@ -28,6 +33,18 @@ static pthread_key_t isolate_key = 0x7777; static pthread_key_t thread_id_key = 0x7777; static pthread_key_t thread_data_key = 0x7777; +/** + * Qualia's node build exports `v8_qualia_set_thread_stack_start(void*)`. V8 >= 12 records the + * start of the OS thread's stack when an isolate is entered and cppgc conservatively scans from + * the current stack pointer up to it during GC; on a coroutine stack that range is garbage. When + * the symbol exists we tell V8 which stack is running on every switch (nullptr = the thread's own + * stack). Resolved with dlsym so this addon still loads on a node without the patch. + */ +typedef void (*set_thread_stack_start_t)(void*); +static set_thread_stack_start_t set_thread_stack_start = NULL; + +static void notify_stack_start(const Coroutine& next); + static size_t stack_size = 0; static size_t coroutines_created_ = 0; static vector fiber_pool; @@ -75,66 +92,113 @@ namespace v8 { } #endif +struct tls_snapshot_t { + v8::Isolate* isolate; + std::vector values; +}; + +/** + * Runs on a fresh helper thread: enter the isolate (which makes V8 assign this thread a new + * ThreadId and thread data) and copy every pthread TLS slot below `coro_thread_key`. + */ #ifndef WINDOWS -static void* find_thread_id_key(void* arg) +static void* snapshot_tls(void* arg) #else -static DWORD __stdcall find_thread_id_key(LPVOID arg) +static DWORD __stdcall snapshot_tls(LPVOID arg) #endif { - v8::Isolate* isolate = static_cast(arg); - assert(isolate != NULL); - v8::Locker locker(isolate); - isolate->Enter(); - - // First pass-- find isolate thread key + tls_snapshot_t* snap = static_cast(arg); + assert(snap->isolate != NULL); + v8::Locker locker(snap->isolate); + snap->isolate->Enter(); #ifdef __MUSL__ // 128 is default max key in musl - for (pthread_key_t ii = 1; ii < 128; ++ii) { + const pthread_key_t key_count = 128; #else - for (pthread_key_t ii = coro_thread_key; ii > 0; --ii) { + const pthread_key_t key_count = coro_thread_key; #endif - void* tls = pthread_getspecific(ii - 1); - if (tls == isolate) { + snap->values.assign(key_count, NULL); + for (pthread_key_t ii = 0; ii < key_count; ++ii) { + snap->values[ii] = pthread_getspecific(ii); + } + snap->isolate->Exit(); + return NULL; +} + +static void take_tls_snapshot(v8::Isolate* isolate, tls_snapshot_t& snap) { + snap.isolate = isolate; + pthread_t thread; + pthread_create(&thread, NULL, snapshot_tls, &snap); + pthread_join(thread, NULL); +} + +/** + * Locate the pthread TLS keys V8 uses for the current isolate, per-isolate thread data and + * ThreadId. Up to V8 12 all three were pthread keys and can be found by value (the isolate + * pointer, a struct whose first word is the isolate pointer, and the int thread id read from + * that struct). Newer V8 keeps the isolate and thread data in compiler thread_local storage + * (which Locker/Unlocker maintain for us), so only the ThreadId key matters: it is the slot + * holding a small positive int that increments between two consecutively created threads. + */ +static void find_thread_id_key(v8::Isolate* isolate) { + tls_snapshot_t a; + take_tls_snapshot(isolate, a); + const pthread_key_t key_count = a.values.size(); + + // First pass-- find isolate thread key + for (pthread_key_t ii = key_count; ii > 0; --ii) { + if (a.values[ii - 1] == isolate) { isolate_key = ii - 1; break; } } - assert(isolate_key != 0x7777); // Second pass-- find data key int thread_id = 0; -#ifdef __MUSL__ - for (pthread_key_t ii = 0; ii < 128; ++ii) { -#else - for (pthread_key_t ii = isolate_key + 1; ii < coro_thread_key; ++ii) { -#endif - void* tls = pthread_getspecific(ii); - if (can_poke(tls) && *(void**)tls == isolate) { - // First member of per-thread data is the isolate - thread_data_key = ii; - // Second member is the thread id - thread_id = *(int*)((void**)tls + 1); - break; + if (isolate_key != 0x7777) { + for (pthread_key_t ii = isolate_key + 1; ii < key_count; ++ii) { + void* tls = a.values[ii]; + if (can_poke(tls) && *(void**)tls == isolate) { + // First member of per-thread data is the isolate + thread_data_key = ii; + // Second member is the thread id + thread_id = *(int*)((void**)tls + 1); + break; + } } } - assert(thread_data_key != 0x7777); // Third pass-- find thread id key -#ifdef __MUSL__ - for (pthread_key_t ii = 0; ii < 128; ++ii) { -#else - for (pthread_key_t ii = isolate_key + 1; ii < coro_thread_key; ++ii) { -#endif - int tls = static_cast(reinterpret_cast(pthread_getspecific(ii))); - if (tls == thread_id) { - thread_id_key = ii; - break; + if (thread_data_key != 0x7777) { + for (pthread_key_t ii = isolate_key + 1; ii < key_count; ++ii) { + int tls = static_cast(reinterpret_cast(a.values[ii])); + if (tls == thread_id) { + thread_id_key = ii; + break; + } } } - assert(thread_id_key != 0x7777); - isolate->Exit(); - return NULL; + if (thread_id_key == 0x7777) { + // Fallback for V8 >= 13: compare against a second helper thread. V8 hands out thread ids + // from an atomic counter, so the ThreadId slot is the one that reads n in the first thread + // and n + 1 in the second. + tls_snapshot_t b; + take_tls_snapshot(isolate, b); + for (pthread_key_t ii = 0; ii < key_count && ii < b.values.size(); ++ii) { + intptr_t va = reinterpret_cast(a.values[ii]); + intptr_t vb = reinterpret_cast(b.values[ii]); + if (va > 0 && va < (1 << 24) && vb == va + 1) { + thread_id_key = ii; + break; + } + } + } + assert(thread_id_key != 0x7777); + if (getenv("FIBERS_DEBUG_TLS")) { + fprintf(stderr, "fibers: v8 tls keys isolate=%d thread_data=%d thread_id=%d (0x7777 = not found)\n", + (int)isolate_key, (int)thread_data_key, (int)thread_id_key); + } } /** @@ -149,9 +213,13 @@ void Coroutine::init(v8::Isolate* isolate) { thread_data_key = v8::internal::Isolate::per_isolate_thread_data_key_; thread_id_key = v8::internal::Isolate::thread_id_key_; #elif !defined(CORO_PTHREAD) - pthread_t thread; - pthread_create(&thread, NULL, find_thread_id_key, isolate); - pthread_join(thread, NULL); + find_thread_id_key(isolate); +#endif +#if !defined(WINDOWS) && !defined(CORO_PTHREAD) + set_thread_stack_start = reinterpret_cast(dlsym(RTLD_DEFAULT, "v8_qualia_set_thread_stack_start")); + if (getenv("FIBERS_DEBUG_TLS")) { + fprintf(stderr, "fibers: v8_qualia_set_thread_stack_start %s\n", set_thread_stack_start ? "found" : "not found (GC may scan the wrong stack)"); + } #endif } @@ -253,18 +321,34 @@ void Coroutine::reset(entry_t* entry, void* arg) { this->arg = arg; } +static void notify_stack_start(const Coroutine& next) { + if (!set_thread_stack_start) { + return; + } + void* bottom = next.bottom(); + // The original thread's Coroutine has no stack of its own: fall back to the OS thread's stack. + set_thread_stack_start(bottom ? static_cast(bottom) + next.stack_bytes() : NULL); +} + void Coroutine::transfer(Coroutine& next) { assert(this != &next); #ifndef CORO_PTHREAD - fls_data[0] = pthread_getspecific(isolate_key); - fls_data[1] = pthread_getspecific(thread_id_key); - fls_data[2] = pthread_getspecific(thread_data_key); - - pthread_setspecific(isolate_key, next.fls_data[0]); - pthread_setspecific(thread_id_key, next.fls_data[1]); - pthread_setspecific(thread_data_key, next.fls_data[2]); + // Keys V8 no longer keeps in pthread TLS stay at 0x7777 and are skipped. + if (isolate_key != 0x7777) { + fls_data[0] = pthread_getspecific(isolate_key); + pthread_setspecific(isolate_key, next.fls_data[0]); + } + if (thread_id_key != 0x7777) { + fls_data[1] = pthread_getspecific(thread_id_key); + pthread_setspecific(thread_id_key, next.fls_data[1]); + } + if (thread_data_key != 0x7777) { + fls_data[2] = pthread_getspecific(thread_data_key); + pthread_setspecific(thread_data_key, next.fls_data[2]); + } pthread_setspecific(coro_thread_key, &next); + notify_stack_start(next); #endif coro_transfer(&context, &next.context); #ifndef CORO_PTHREAD @@ -322,6 +406,14 @@ void* Coroutine::bottom() const { #endif } +size_t Coroutine::stack_bytes() const { +#ifdef CORO_FIBER + return stack_size * sizeof(void*); +#else + return stack.ssze; +#endif +} + size_t Coroutine::size() const { return sizeof(Coroutine) + stack_size * sizeof(void*); } diff --git a/src/coroutine.h b/src/coroutine.h index bc25ed5..8a16b25 100644 --- a/src/coroutine.h +++ b/src/coroutine.h @@ -88,6 +88,11 @@ class Coroutine { */ void* bottom() const; + /** + * Size in bytes of this coroutine's stack (0 for the original thread). + */ + size_t stack_bytes() const; + /** * Returns the size this Coroutine takes up in the heap. */ From 2fb6179e25c2e466b8c0ec9c9fc5454501898806 Mon Sep 17 00:00:00 2001 From: Jordan Baczuk Date: Wed, 16 Sep 2026 09:04:15 -0600 Subject: [PATCH 4/4] fix: make V8 ThreadId key discovery tolerate concurrent thread creation and fail loudly The V8 >= 13 fallback compared two helper-thread TLS snapshots and required the ThreadId slot to read exactly n and n + 1. V8 creates platform worker threads lazily, so a thread created between the two snapshots consumes an id and the match fails; observed about once per 80 loads on node 24. The comparison now accepts a gap of up to 64, requires the matching key to be unique, and retries with a fresh snapshot pair up to eight times (150/150 loads afterwards). A failed discovery is now a fatal error with a message instead of an assert: the assert is compiled out of Release builds, and without the key every coroutine shares the OS thread's V8 ThreadId, so Locker/Unlocker archiving corrupts JS stacks intermittently later. Co-Authored-By: Claude Fable 5.1 --- src/coroutine.cc | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/coroutine.cc b/src/coroutine.cc index dd69b6a..ab96594 100644 --- a/src/coroutine.cc +++ b/src/coroutine.cc @@ -180,21 +180,37 @@ static void find_thread_id_key(v8::Isolate* isolate) { } if (thread_id_key == 0x7777) { - // Fallback for V8 >= 13: compare against a second helper thread. V8 hands out thread ids - // from an atomic counter, so the ThreadId slot is the one that reads n in the first thread - // and n + 1 in the second. - tls_snapshot_t b; - take_tls_snapshot(isolate, b); - for (pthread_key_t ii = 0; ii < key_count && ii < b.values.size(); ++ii) { - intptr_t va = reinterpret_cast(a.values[ii]); - intptr_t vb = reinterpret_cast(b.values[ii]); - if (va > 0 && va < (1 << 24) && vb == va + 1) { - thread_id_key = ii; - break; + // Fallback for V8 >= 13: compare against further helper threads. V8 hands out thread ids + // from an atomic counter, so the ThreadId slot is the one whose small positive value grows + // between two consecutively created threads. Other threads (V8 platform workers) may be + // created concurrently and consume ids, so allow a small gap, require the match to be + // unique, and retry with a fresh snapshot pair when it is not. + for (int attempt = 0; attempt < 8 && thread_id_key == 0x7777; ++attempt) { + tls_snapshot_t b; + take_tls_snapshot(isolate, b); + pthread_key_t candidate = 0x7777; + int candidates = 0; + for (pthread_key_t ii = 0; ii < key_count && ii < b.values.size(); ++ii) { + intptr_t va = reinterpret_cast(a.values[ii]); + intptr_t vb = reinterpret_cast(b.values[ii]); + if (va > 0 && va < (1 << 24) && vb > va && vb - va <= 64) { + candidate = ii; + ++candidates; + } + } + if (candidates == 1) { + thread_id_key = candidate; } + a = b; } } - assert(thread_id_key != 0x7777); + if (thread_id_key == 0x7777) { + // Without this key every coroutine shares the OS thread's V8 ThreadId and Locker/Unlocker + // archiving silently corrupts JS stacks. Refuse to run rather than fail intermittently + // later (an assert would be compiled out of Release builds). + fprintf(stderr, "fibers: could not locate V8's ThreadId thread-local key; this node/V8 build is not supported (set FIBERS_DEBUG_TLS=1 for details)\n"); + abort(); + } if (getenv("FIBERS_DEBUG_TLS")) { fprintf(stderr, "fibers: v8 tls keys isolate=%d thread_data=%d thread_id=%d (0x7777 = not found)\n", (int)isolate_key, (int)thread_data_key, (int)thread_id_key);