Run FinalizationRegistry callback in the job queue

The spec says HostMakeJobCallback has to be used on the callback: https://tc39.es/ecma262/multipage/managing-memory.html#sec-finalization-registry-cleanup-callback

That makes the following (arguably contrived) example run forever until
memory is exhausted.

```js
let count = 0;
function main() {
    console.log(`main! ${++count}`);
    const registry = new FinalizationRegistry(() => {
        globalThis.foo = main();
    });
    registry.register([]);
    registry.register([]);
    return registry;
}
main();

console.log(count);
```

That is unlike V8, which runs 0 times. This can be explained by the
difference in GC implementations and since FinRec makes GC observable,
here we are!

Fixes: https://github.com/quickjs-ng/quickjs/issues/432
This commit is contained in:
Saúl Ibarra Corretgé 2024-09-06 09:42:21 +02:00
parent c740aa07c1
commit 61c8fe6fb0
2 changed files with 26 additions and 10 deletions

View file

@ -248,6 +248,8 @@ struct JSRuntime {
BOOL in_out_of_memory : 8; BOOL in_out_of_memory : 8;
/* and likewise if inside Error.prepareStackTrace() */ /* and likewise if inside Error.prepareStackTrace() */
BOOL in_prepare_stack_trace : 8; BOOL in_prepare_stack_trace : 8;
/* true if inside JS_FreeRuntime */
BOOL in_free : 8;
struct JSStackFrame *current_stack_frame; struct JSStackFrame *current_stack_frame;
@ -1812,6 +1814,8 @@ int JS_EnqueueJob(JSContext *ctx, JSJobFunc *job_func,
JSJobEntry *e; JSJobEntry *e;
int i; int i;
assert(!rt->in_free);
e = js_malloc(ctx, sizeof(*e) + argc * sizeof(JSValue)); e = js_malloc(ctx, sizeof(*e) + argc * sizeof(JSValue));
if (!e) if (!e)
return -1; return -1;
@ -1932,6 +1936,7 @@ void JS_FreeRuntime(JSRuntime *rt)
struct list_head *el, *el1; struct list_head *el, *el1;
int i; int i;
rt->in_free = TRUE;
JS_FreeValueRT(rt, rt->current_exception); JS_FreeValueRT(rt, rt->current_exception);
list_for_each_safe(el, el1, &rt->job_list) { list_for_each_safe(el, el1, &rt->job_list) {
@ -53228,6 +53233,13 @@ static const JSClassShortDef js_finrec_class_def[] = {
{ JS_ATOM_FinalizationRegistry, js_finrec_finalizer, js_finrec_mark }, /* JS_CLASS_FINALIZATION_REGISTRY */ { JS_ATOM_FinalizationRegistry, js_finrec_finalizer, js_finrec_mark }, /* JS_CLASS_FINALIZATION_REGISTRY */
}; };
static JSValue js_finrec_job(JSContext *ctx, int argc, JSValue *argv)
{
JSValue ret = JS_Call(ctx, argv[0], JS_UNDEFINED, 1, &argv[1]);
JS_FreeValue(ctx, argv[1]);
return ret;
}
void JS_AddIntrinsicWeakRef(JSContext *ctx) void JS_AddIntrinsicWeakRef(JSContext *ctx)
{ {
JSRuntime *rt = ctx->rt; JSRuntime *rt = ctx->rt;
@ -53310,12 +53322,11 @@ static void reset_weak_ref(JSRuntime *rt, JSWeakRefRecord **first_weak_ref)
/** /**
* During the GC sweep phase the held object might be collected first. * During the GC sweep phase the held object might be collected first.
*/ */
if (!JS_IsObject(fre->held_val) || JS_IsLiveObject(frd->ctx->rt, fre->held_val)) { if (!rt->in_free && (!JS_IsObject(fre->held_val) || JS_IsLiveObject(rt, fre->held_val))) {
JSValue func = js_dup(frd->cb); JSValue args[2];
JSValue ret = JS_Call(frd->ctx, func, JS_UNDEFINED, 1, &fre->held_val); args[0] = frd->cb;
JS_FreeValueRT(rt, func); args[1] = fre->held_val;
JS_FreeValueRT(rt, ret); JS_EnqueueJob(frd->ctx, js_finrec_job, 2, args);
JS_FreeValueRT(rt, fre->held_val);
} }
JS_FreeValueRT(rt, fre->token); JS_FreeValueRT(rt, fre->token);
js_free_rt(rt, fre); js_free_rt(rt, fre);

View file

@ -998,14 +998,19 @@ function test_finalization_registry()
let expected = {}; let expected = {};
let actual; let actual;
let finrec = new FinalizationRegistry(v => { actual = v }); let finrec = new FinalizationRegistry(v => { actual = v });
finrec.register({}, expected); // {} goes out of scope immediately finrec.register({}, expected);
queueMicrotask(() => {
assert(actual, expected); assert(actual, expected);
});
} }
{ {
let expected = 42;
let actual; let actual;
let finrec = new FinalizationRegistry(v => { actual = v }); let finrec = new FinalizationRegistry(v => { actual = v });
finrec.register({}, 42); // {} goes out of scope immediately finrec.register({}, expected);
assert(actual, 42); queueMicrotask(() => {
assert(actual, expected);
});
} }
} }