diff --git a/quickjs.c b/quickjs.c index af401ae37..8922d6c9e 100644 --- a/quickjs.c +++ b/quickjs.c @@ -31686,6 +31686,35 @@ static int js_inner_module_linking(JSContext *ctx, JSModuleDef *m, return -1; } +/* Release the variable references taken by a failed link of 'm', so + that linking it again does not leak them. */ +static void js_reset_module_var_refs(JSContext *ctx, JSModuleDef *m) +{ + JSVarRef **var_refs; + JSObject *p; + int i; + + if (m->init_func != NULL) + return; /* the exports of a C module are not set by linking */ + p = JS_VALUE_GET_OBJ(m->func_obj); + var_refs = p->u.func.var_refs; + for(i = 0; i < m->import_entries_count; i++) { + JSImportEntry *mi = &m->import_entries[i]; + /* a namespace import is a variable of the module itself */ + if (mi->import_name != JS_ATOM__star_) { + free_var_ref(ctx->rt, var_refs[mi->var_idx]); + var_refs[mi->var_idx] = NULL; + } + } + for(i = 0; i < m->export_entries_count; i++) { + JSExportEntry *me = &m->export_entries[i]; + if (me->export_type == JS_EXPORT_TYPE_LOCAL) { + free_var_ref(ctx->rt, me->u.local.var_ref); + me->u.local.var_ref = NULL; + } + } +} + /* Prepare a module to be executed by resolving all the imported variables. */ static int js_link_module(JSContext *ctx, JSModuleDef *m) @@ -31708,6 +31737,7 @@ static int js_link_module(JSContext *ctx, JSModuleDef *m) m1 = stack_top; assert(m1->status == JS_MODULE_STATUS_LINKING); m1->status = JS_MODULE_STATUS_UNLINKED; + js_reset_module_var_refs(ctx, m1); stack_top = m1->stack_prev; } return -1; diff --git a/tests.conf b/tests.conf index 192656e0b..6e2b21b11 100644 --- a/tests.conf +++ b/tests.conf @@ -16,3 +16,5 @@ tests/fixture_throwing_module.js tests/fixture_reexport_source.js tests/fixture_reexport_missing.js tests/fixture_reexport_direct.js +tests/fixture_link_fail_a.js +tests/fixture_link_fail_b.js diff --git a/tests/bug1833.js b/tests/bug1833.js new file mode 100644 index 000000000..78f4d45ad --- /dev/null +++ b/tests/bug1833.js @@ -0,0 +1,14 @@ +// https://github.com/quickjs-ng/quickjs/issues/1833 +// A module whose linking fails is reset to unlinked and linked again by the +// next import. The variable references taken by the failed link must be +// released, otherwise they leak and JS_FreeRuntime asserts in debug builds. +import { assert } from "./assert.js"; + +const results = await Promise.allSettled([ + import("./fixture_link_fail_a.js"), + import("./fixture_link_fail_a.js"), +]); +for (const r of results) { + assert(r.status, "rejected"); + assert(r.reason instanceof SyntaxError); +} diff --git a/tests/fixture_link_fail_a.js b/tests/fixture_link_fail_a.js new file mode 100644 index 000000000..691fc28f5 --- /dev/null +++ b/tests/fixture_link_fail_a.js @@ -0,0 +1,5 @@ +// Linking fails on the last import, after the other imports of this module +// and the imports and exports of fixture_link_fail_b.js have been resolved. +import { b, ns } from "./fixture_link_fail_b.js"; +import { missing } from "./fixture_link_fail_b.js"; +export const a = 1; diff --git a/tests/fixture_link_fail_b.js b/tests/fixture_link_fail_b.js new file mode 100644 index 000000000..fccbc41ec --- /dev/null +++ b/tests/fixture_link_fail_b.js @@ -0,0 +1,3 @@ +import { a } from "./fixture_link_fail_a.js"; +export const b = 2; +export * as ns from "./fixture_reexport_source.js";