Node 24 / V8 13 compatibility with CORO_PTHREAD coroutines

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 {

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);
@@ -27,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);

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) {

src/fibers.cc

@@ -202,11 +202,13 @@ namespace uni {
 		args.GetReturnValue().Set(handle);
 	}
 	template <class T>
-	void Return(Local<T> 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<T> handle, const GetterCallbackInfo& info) {
 		info.GetReturnValue().Set(handle);
 	}
 	template <class T>
-	void Return(Persistent<T>& handle, GetterCallbackInfo info) {
+	void Return(Persistent<T>& handle, const GetterCallbackInfo& info) {
 		info.GetReturnValue().Set(Local<T>::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<Object> Holder(const FunctionCallbackInfo<Value>& args) { return args.This(); }
+#else
+	inline Local<Object> Holder(const FunctionCallbackInfo<Value>& 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> object, Local<String> name,
+		FunctionType (*getter)(Local<String>, const GetterCallbackInfo&),
+		void (*setter)(Local<String> property, Local<Value> 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> object, Local<String> name,
 		FunctionType (*getter)(Local<String>, const GetterCallbackInfo&),
@@ -462,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<WeakCallback>(isolate, handle, (void*)this);
 		}
 
@@ -470,6 +489,7 @@ class Fiber {
 		 * i.e. While running.
 		 */
 		void ClearWeak() {
+			if (handle.IsEmpty()) return; // see MakeWeak()
 			handle.ClearWeak();
 		}
 
@@ -490,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;
 			}
 
@@ -532,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
 			}
 		}
 
@@ -560,7 +592,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 +633,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 +654,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);
@@ -833,7 +865,8 @@ class Fiber {
 		}
 
 		static uni::FunctionType GetCurrent(Local<String> 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);
@@ -891,7 +924,11 @@ class Fiber {
 				uni::NewFunctionTemplate(isolate, Run, Local<Value>(), sig));
 			proto->Set(uni::NewLatin1Symbol(isolate, "throwInto"),
 				uni::NewFunctionTemplate(isolate, ThrowInto, Local<Value>(), 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<Function> yield = uni::GetFunction(uni::NewFunctionTemplate(isolate, Yield_));

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);