From f76f1308c2e676cf93378abacc6b73f6c68ef6a3 Mon Sep 17 00:00:00 2001 From: Arshia Ghafoori Date: Tue, 1 Sep 2026 11:31:23 +0000 Subject: [PATCH] Report a dead target as empty from napi_get_reference_value QuickJS frees reference cycles in two passes, so a finalizer can still see other members of its own cycle as zombies -- that is what JS_IsLiveObject exists for. A weak reference slot pointing at such an object has not been cleared yet, because reset_weak_ref only runs from that object's own free_object. napi_get_reference_value would therefore dup_inner() a zombie, handing the caller a fresh reference to memory the collector is about to release; whichever scope owns the resulting napi_value then frees it again and the process trips a heap-use-after-free. Report such a target as empty instead, which is what callers already handle for a collected weak reference. The check is deliberately restricted to objects. JS_IsLiveObject reports every non-object -- strings, symbols, bigints -- as not live, so applying it unconditionally makes every reference to one read back as empty (51 node:crypto tests fail that way). Co-Authored-By: Claude Opus 5 (1M context) --- quickjs/src/js_native_api_quickjs.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/quickjs/src/js_native_api_quickjs.cc b/quickjs/src/js_native_api_quickjs.cc index cbd1f48..6c05096 100644 --- a/quickjs/src/js_native_api_quickjs.cc +++ b/quickjs/src/js_native_api_quickjs.cc @@ -2880,6 +2880,22 @@ extern "C" *result = nullptr; return napi_ok; } + // QuickJS frees reference cycles in two passes, so a finalizer can still + // see other members of its cycle as zombies (hence JS_IsLiveObject). A weak + // slot pointing at one has not been cleared yet -- reset_weak_ref only runs + // in that object's own free_object -- so dup_inner() here would resurrect + // memory the collector is about to release, and the scope that owns the + // resulting napi_value would later free it again. Report it as gone. + // Objects only: JS_IsLiveObject reports every non-object (strings, symbols, + // bigints) as "not live", which would make every reference to one read back + // as empty. + JSValueConst inner = slot->get_inner(); + if (JS_IsObject(inner) && slot->context() != nullptr && + !JS_IsLiveObject(JS_GetRuntime(slot->context()), inner)) + { + *result = nullptr; + return napi_ok; + } *result = env->wrap_value_in_current_scope(slot->context(), slot->dup_inner(), true); return (*result == nullptr) ? napi_generic_failure : napi_ok; }