Skip to content

Commit 5cb2c21

Browse files
committedSep 9, 2024
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: #432
1 parent ad834a1 commit 5cb2c21

File tree

2 files changed

+26
-10
lines changed

2 files changed

+26
-10
lines changed
 

‎quickjs.c

+17-6
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,8 @@ struct JSRuntime {
248248
BOOL in_out_of_memory : 8;
249249
/* and likewise if inside Error.prepareStackTrace() */
250250
BOOL in_prepare_stack_trace : 8;
251+
/* true if inside JS_FreeRuntime */
252+
BOOL in_free : 8;
251253

252254
struct JSStackFrame *current_stack_frame;
253255

@@ -1812,6 +1814,8 @@ int JS_EnqueueJob(JSContext *ctx, JSJobFunc *job_func,
18121814
JSJobEntry *e;
18131815
int i;
18141816

1817+
assert(!rt->in_free);
1818+
18151819
e = js_malloc(ctx, sizeof(*e) + argc * sizeof(JSValue));
18161820
if (!e)
18171821
return -1;
@@ -1932,6 +1936,7 @@ void JS_FreeRuntime(JSRuntime *rt)
19321936
struct list_head *el, *el1;
19331937
int i;
19341938

1939+
rt->in_free = TRUE;
19351940
JS_FreeValueRT(rt, rt->current_exception);
19361941

19371942
list_for_each_safe(el, el1, &rt->job_list) {
@@ -53228,6 +53233,13 @@ static const JSClassShortDef js_finrec_class_def[] = {
5322853233
{ JS_ATOM_FinalizationRegistry, js_finrec_finalizer, js_finrec_mark }, /* JS_CLASS_FINALIZATION_REGISTRY */
5322953234
};
5323053235

53236+
static JSValue js_finrec_job(JSContext *ctx, int argc, JSValue *argv)
53237+
{
53238+
JSValue ret = JS_Call(ctx, argv[0], JS_UNDEFINED, 1, &argv[1]);
53239+
JS_FreeValue(ctx, argv[1]);
53240+
return ret;
53241+
}
53242+
5323153243
void JS_AddIntrinsicWeakRef(JSContext *ctx)
5323253244
{
5323353245
JSRuntime *rt = ctx->rt;
@@ -53310,12 +53322,11 @@ static void reset_weak_ref(JSRuntime *rt, JSWeakRefRecord **first_weak_ref)
5331053322
/**
5331153323
* During the GC sweep phase the held object might be collected first.
5331253324
*/
53313-
if (!JS_IsObject(fre->held_val) || JS_IsLiveObject(frd->ctx->rt, fre->held_val)) {
53314-
JSValue func = js_dup(frd->cb);
53315-
JSValue ret = JS_Call(frd->ctx, func, JS_UNDEFINED, 1, &fre->held_val);
53316-
JS_FreeValueRT(rt, func);
53317-
JS_FreeValueRT(rt, ret);
53318-
JS_FreeValueRT(rt, fre->held_val);
53325+
if (!rt->in_free && (!JS_IsObject(fre->held_val) || JS_IsLiveObject(rt, fre->held_val))) {
53326+
JSValue args[2];
53327+
args[0] = frd->cb;
53328+
args[1] = fre->held_val;
53329+
JS_EnqueueJob(frd->ctx, js_finrec_job, 2, args);
5331953330
}
5332053331
JS_FreeValueRT(rt, fre->token);
5332153332
js_free_rt(rt, fre);

‎tests/test_builtin.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -998,14 +998,19 @@ function test_finalization_registry()
998998
let expected = {};
999999
let actual;
10001000
let finrec = new FinalizationRegistry(v => { actual = v });
1001-
finrec.register({}, expected); // {} goes out of scope immediately
1002-
assert(actual, expected);
1001+
finrec.register({}, expected);
1002+
queueMicrotask(() => {
1003+
assert(actual, expected);
1004+
});
10031005
}
10041006
{
1007+
let expected = 42;
10051008
let actual;
10061009
let finrec = new FinalizationRegistry(v => { actual = v });
1007-
finrec.register({}, 42); // {} goes out of scope immediately
1008-
assert(actual, 42);
1010+
finrec.register({}, expected);
1011+
queueMicrotask(() => {
1012+
assert(actual, expected);
1013+
});
10091014
}
10101015
}
10111016

0 commit comments

Comments
 (0)