Skip to content

Commit 5d1fd76

Browse files
QuickJS: skip newTarget self-dup for non-constructor functions
ExternalCallback::newTarget is only consulted when the function is invoked as a constructor (Callback(), isConstructCall-gated). For regular functions created via napi_create_function (i.e. magic=0 — all Napi::Function::New() lambdas, polyfill callbacks, etc.) the newTarget self-dup creates a useless cycle: func -> func_data[callbackData] -> opaque ExternalCallback -> newTarget -> func NapiCallback's gc_mark exposes the newTarget to QuickJS's cycle collector, so this is technically safe — but in practice many of these cycles end up externally pinned (e.g. via bytecode closures or refs captured by polyfills) and survive teardown, contributing to the gc_obj_list residents that trip the assert in JS_FreeRuntime. Skipping the dup for magic=0 eliminates the cycle entirely for the common case (all Napi::Function::New lambdas). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4502c2c commit 5d1fd76

1 file changed

Lines changed: 11 additions & 4 deletions

File tree

Core/Node-API/Source/js_native_api_quickjs.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -954,11 +954,18 @@ static napi_status create_function_internal(napi_env env, const char* utf8name,
954954
// dup'd it into func's data array, so it stays alive as long as func does.
955955
JS_FreeValue(env->context, callbackData);
956956

957-
// newTarget is a strong dup of the function. This creates a cycle
957+
// newTarget is a strong dup of the function — but it's only read for
958+
// constructor calls (see Callback() above, isConstructCall-gated path).
959+
// For regular functions (magic=0) we'd be allocating a useless self-cycle
958960
// func -> func_data[callbackData] -> opaque ExternalCallback -> newTarget -> func
959-
// which is safe because NapiCallback has a gc_mark (see ExternalCallback::GcMark)
960-
// that allows QuickJS's cycle collector to detect and break it.
961-
externalCallback->newTarget = JS_DupValue(env->context, func);
961+
// for every Napi::Function::New call (lambdas, mocha callbacks, console.log,
962+
// etc.). Even with NapiCallback's gc_mark exposing newTarget to the cycle
963+
// collector, in practice many of these cycles end up externally pinned (e.g.
964+
// by closures captured into bytecode) and survive teardown, leaving entries
965+
// on rt->gc_obj_list and tripping the assert in JS_FreeRuntime.
966+
if (magic == 1) {
967+
externalCallback->newTarget = JS_DupValue(env->context, func);
968+
}
962969

963970
*result = FromJSValue(env, func);
964971
napi_clear_last_error(env);

0 commit comments

Comments
 (0)