commit 10cbb55fd86a69ba7fbf8b4d5575b59b862231c2
parent 7bb7a3e78a9baa176297d71d81a42aa2072a7824
Author: Florian Dold <florian@dold.me>
Date: Mon, 10 Mar 2025 19:09:10 +0100
Revert "quickjs: fix garbage collection / var ref bug"
This reverts commit f6b5ac20dca2e40c4eb210fb0a4548e04bf69329.
Diffstat:
5 files changed, 1 insertion(+), 220 deletions(-)
diff --git a/quickjs/quickjs.c b/quickjs/quickjs.c
@@ -338,10 +338,6 @@ typedef struct JSStackFrame {
/* only used in generators. Current stack pointer value. NULL if
the function is running. */
JSValue *cur_sp;
- /* only used in async/generator functions to save the variable references
- that have been evicted to the heap. */
- struct JSVarRef **closed_var_refs;
- int closed_var_refs_count;
} JSStackFrame;
typedef enum {
@@ -5489,8 +5485,7 @@ void __JS_FreeValueRT(JSRuntime *rt, JSValue v)
if (tag == JS_TAG_OBJECT) {
JS_DumpObject(rt, JS_VALUE_GET_OBJ(v));
} else {
- printf("[<short value>]");
- //JS_DumpValueShort(rt, v);
+ JS_DumpValueShort(rt, v);
printf("\n");
}
}
@@ -15985,94 +15980,6 @@ static int js_op_define_class(JSContext *ctx, JSValue *sp,
return -1;
}
-/* Re-attach variable refs to the local,
- * on-stack variables when resuming an async/generator function. */
-static void reopen_var_refs(JSRuntime *rt, JSStackFrame *sf)
-{
- int i;
- JSVarRef *var_ref;
- int var_idx;
-
- for (i = 0; i < sf->closed_var_refs_count; i++) {
- var_ref = sf->closed_var_refs[i];
- sf->closed_var_refs[i] = NULL;
- if (!var_ref) {
- continue;
- }
- var_idx = var_ref->var_idx;
- assert(var_ref->is_detached);
-
- var_ref->is_detached = FALSE;
-
- if (var_ref->is_arg) {
- JS_FreeValueRT(rt, sf->arg_buf[var_idx]);
- sf->arg_buf[var_idx] = var_ref->value;
- var_ref->pvalue = &sf->arg_buf[var_idx];
- } else {
- JS_FreeValueRT(rt, sf->var_buf[var_idx]);
- sf->var_buf[var_idx] = var_ref->value;
- var_ref->pvalue = &sf->var_buf[var_idx];
- }
- remove_gc_object(&var_ref->header);
- list_add_tail(&var_ref->header.link, &sf->var_ref_list);
- free_var_ref(rt, var_ref);
- }
-}
-
-static void close_var_refs_saving(JSRuntime *rt, JSStackFrame *sf)
-{
- struct list_head *el, *el1;
- JSVarRef **var_refs;
- JSVarRef *var_ref;
- int var_idx;
- int count = 0;
- int i;
-
- if (list_empty(&sf->var_ref_list)) {
- return;
- }
-
- list_for_each_safe(el, el1, &sf->var_ref_list) {
- count++;
- }
- var_refs = sf->closed_var_refs;
- if (count > sf->closed_var_refs_count) {
- var_refs = js_realloc_rt(rt, var_refs, count * sizeof(var_refs[0]));
- if (!var_refs) {
- // FIXME
- abort();
- }
- for (i = sf->closed_var_refs_count; i < count; i++) {
- var_refs[i] = NULL;
- }
- sf->closed_var_refs = var_refs;
- sf->closed_var_refs_count = count;
- }
-
- i = 0;
- list_for_each_safe(el, el1, &sf->var_ref_list) {
- var_ref = list_entry(el, JSVarRef, header.link);
- var_idx = var_ref->var_idx;
- assert(!var_ref->is_detached);
- /* move ownership from stack into varref */
- if (var_ref->is_arg)
- var_ref->value = JS_DupValueRT(rt, sf->arg_buf[var_idx]);
- else
- var_ref->value = JS_DupValueRT(rt, sf->var_buf[var_idx]);
- var_ref->pvalue = &var_ref->value;
- /* the reference is no longer to a local variable */
- var_ref->is_detached = TRUE;
- /* delete from stack frame's var_ref_list */
- list_del(&var_ref->header.link);
- /* prevent from collecting while async/generator is still active */
- var_refs[i] = var_ref;
- var_ref->header.ref_count++;
- add_gc_object(rt, &var_ref->header, JS_GC_OBJ_TYPE_VAR_REF);
- i++;
- }
- assert(list_empty(&sf->var_ref_list));
-}
-
static void close_var_refs(JSRuntime *rt, JSStackFrame *sf)
{
struct list_head *el, *el1;
@@ -16082,7 +15989,6 @@ static void close_var_refs(JSRuntime *rt, JSStackFrame *sf)
list_for_each_safe(el, el1, &sf->var_ref_list) {
var_ref = list_entry(el, JSVarRef, header.link);
var_idx = var_ref->var_idx;
- assert(!var_ref->is_detached);
if (var_ref->is_arg)
var_ref->value = JS_DupValueRT(rt, sf->arg_buf[var_idx]);
else
@@ -16090,8 +15996,6 @@ static void close_var_refs(JSRuntime *rt, JSStackFrame *sf)
var_ref->pvalue = &var_ref->value;
/* the reference is no longer to a local variable */
var_ref->is_detached = TRUE;
- /* delete from stack frame's var_ref_list */
- list_del(&var_ref->header.link);
add_gc_object(rt, &var_ref->header, JS_GC_OBJ_TYPE_VAR_REF);
}
}
@@ -16363,7 +16267,6 @@ static JSValue __JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
pc = sf->cur_pc;
sf->prev_frame = rt->current_stack_frame;
rt->current_stack_frame = sf;
- reopen_var_refs(rt, sf);
if (s->throw_flag)
goto exception;
else
@@ -16396,9 +16299,6 @@ static JSValue __JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
if (js_check_stack_overflow(rt, alloca_size))
return JS_ThrowStackOverflow(caller_ctx);
- sf->closed_var_refs = NULL;
- sf->closed_var_refs_count = 0;
-
sf->js_mode = b->js_mode;
arg_buf = argv;
sf->arg_count = argc;
@@ -18807,7 +18707,6 @@ static JSValue __JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
generator function. */
if (b->func_kind != JS_FUNC_NORMAL) {
done_generator:
- close_var_refs_saving(rt, sf);
sf->cur_pc = pc;
sf->cur_sp = sp;
} else {
@@ -19224,15 +19123,6 @@ static void async_func_mark(JSRuntime *rt, JSAsyncFunctionState *s,
for(sp = sf->arg_buf; sp < sf->cur_sp; sp++)
JS_MarkValue(rt, *sp, mark_func);
}
- if (sf->closed_var_refs)
- {
- int i;
- for (i = 0; i < sf->closed_var_refs_count; i++) {
- if (sf->closed_var_refs[i]) {
- mark_func(rt, &sf->closed_var_refs[i]->header);
- }
- }
- }
}
static void async_func_free(JSRuntime *rt, JSAsyncFunctionState *s)
@@ -19253,18 +19143,6 @@ static void async_func_free(JSRuntime *rt, JSAsyncFunctionState *s)
}
js_free_rt(rt, sf->arg_buf);
}
- if (sf->closed_var_refs)
- {
- int i;
- for (i = 0; i < sf->closed_var_refs_count; i++) {
- if (sf->closed_var_refs[i]) {
- free_var_ref(rt, sf->closed_var_refs[i]);
- }
- }
- js_free_rt(rt, sf->closed_var_refs);
- sf->closed_var_refs = NULL;
- sf->closed_var_refs_count = 0;
- }
JS_FreeValueRT(rt, sf->cur_func);
JS_FreeValueRT(rt, s->this_val);
}
diff --git a/quickjs/tests/crash1.js b/quickjs/tests/crash1.js
@@ -1,16 +0,0 @@
-import * as std from "std";
-
-async function waitUntilDone(ws) {
- let p = {};
- globalThis.escFun = function escFun() { return p; };
- p.promise = new Promise(function promIni() {});
- await p.promise;
-}
-
-waitUntilDone();
-
-//console.log("-------- running gc1");
-//std.gc();
-//console.log("-------- running gc2");
-//std.gc();
-//console.log("-------- end");
diff --git a/quickjs/tests/crash2.js b/quickjs/tests/crash2.js
@@ -1,19 +0,0 @@
-import * as std from "std";
-
-let listener;
-
-async function waitUntilDone(ws) {
- let p2 = {};
- listener = (yn) => {
- return p2;
- };
- p2.promise = new Promise(() => {});
- await p2.promise;
-}
-
-waitUntilDone();
-
-console.log("---- calling gc");
-std.gc();
-console.log("---- calling listener");
-listener();
diff --git a/quickjs/tests/gentest.js b/quickjs/tests/gentest.js
@@ -1,18 +0,0 @@
-function *foo() {
- let x = 42;
- console.log("first yield");
- yield () => x;
- console.log("second yield");
- yield v => { x = v; };
- console.log("my x", x);
-}
-
-const g = foo();
-
-const fun1 = g.next().value;
-const fun2 = g.next().value;
-
-console.log(fun1());
-fun2(41);
-console.log(fun1());
-g.next();
diff --git a/quickjs/tests/promtest3.js b/quickjs/tests/promtest3.js
@@ -1,44 +0,0 @@
-// Running this file crashes vanilla quickjs-2021-03-27
-// due to a use-after-free bug.
-// That happens because innerFun keeps a reference
-// to the local variable p, but p gets erroneously
-// garbage-collected.
-// The outerFun infinitely waits for the promise
-// to finish. Thus, its local variables are kept on the
-// stack. Thus innerFun doesn't directly reference
-// outerFun, and outerFun gets collected
-// by the cycle collector.
-
-import * as os from "os";
-import * as std from "std";
-
-let f;
-
-let promise = new Promise(() => {});
-
-let outerFun = async function outerFun() {
- let p = { foo: "bar" };
- function innerFun() {
- console.log(p.foo);
- }
- f = innerFun;
- // Wait indefinitely
- await promise;
-}
-outerFun();
-
-// directly
-
-//console.log("cleaning up");
-//console.log("deleting promise");
-//promise = undefined;
-//console.log("deleting outerFun");
-//outerFun = undefined;
-//console.log("deleting f");
-//f = undefined;
-//console.log("exiting");
-//std.exit(42);
-
-
-// via timer
-os.setTimeout(() => { console.log("calling GC"); std.gc(); f(); }, 0);
