From 38fa7d7cf6d6be5fbc72df461a72f5db7f14af1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Thu, 11 Apr 2024 10:57:31 +0200 Subject: [PATCH] Fix crash in FinalizationRegistry when the observed object is GC'd In the pathological case shown in https://github.com/quickjs-ng/quickjs/issues/367 both the object and the registry will be destroyed as part of the GC phase of JS_FreeRuntime. When the GC sweep happens it's possible we are holding on to a corpse so avoid calling the registry callback in that case. This is similar to how Weak{Map,Set} deal with iterators being freed as part of a cycle. Fixes: https://github.com/quickjs-ng/quickjs/issues/367 --- quickjs.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/quickjs.c b/quickjs.c index d1706be..75e05cd 100644 --- a/quickjs.c +++ b/quickjs.c @@ -52141,11 +52141,16 @@ static void reset_weak_ref(JSRuntime *rt, JSWeakRefRecord **first_weak_ref) fre = wr->u.fin_rec_entry; JSFinalizationRegistryData *frd = JS_GetOpaque(fre->obj, JS_CLASS_FINALIZATION_REGISTRY); assert(frd != NULL); - JSValue func = js_dup(frd->cb); - JSValue ret = JS_Call(frd->ctx, func, JS_UNDEFINED, 1, &fre->held_val); - JS_FreeValueRT(rt, func); - JS_FreeValueRT(rt, ret); - JS_FreeValueRT(rt, fre->held_val); + /** + * During the GC sweep phase the held object might be collected first. + */ + if (JS_IsLiveObject(frd->ctx->rt, fre->held_val)) { + JSValue func = js_dup(frd->cb); + JSValue ret = JS_Call(frd->ctx, func, JS_UNDEFINED, 1, &fre->held_val); + JS_FreeValueRT(rt, func); + JS_FreeValueRT(rt, ret); + JS_FreeValueRT(rt, fre->held_val); + } JS_FreeValueRT(rt, fre->token); js_free_rt(rt, fre); break;