diff --git a/quickjs.c b/quickjs.c index af401ae37..6208e9b23 100644 --- a/quickjs.c +++ b/quickjs.c @@ -53868,12 +53868,20 @@ static void js_map_finalizer(JSRuntime *rt, JSValueConst val) if (s) { /* if the object is deleted we are sure that no iterator is using it */ + if (s->is_weak) { + /* first pass to remove the weak references, so that freeing + a value (e.g. a symbol that is also a key of this map) + cannot reset and free records we have yet to visit */ + list_for_each(el, &s->records) { + mr = list_entry(el, JSMapRecord, link); + if (!mr->empty) + delete_map_weak_ref(rt, mr); + } + } list_for_each_safe(el, el1, &s->records) { mr = list_entry(el, JSMapRecord, link); if (!mr->empty) { - if (s->is_weak) - delete_map_weak_ref(rt, mr); - else + if (!s->is_weak) JS_FreeValueRT(rt, mr->key); JS_FreeValueRT(rt, mr->value); } diff --git a/tests/bug1835.js b/tests/bug1835.js new file mode 100644 index 000000000..acdef3e8e --- /dev/null +++ b/tests/bug1835.js @@ -0,0 +1,36 @@ +// https://github.com/quickjs-ng/quickjs/issues/1835 +// Freeing a WeakMap whose values are symbols that are also its keys crashed +// in the WeakMap finalizer. Run under ASAN. +import { assert } from "./assert.js"; + +function f() { + var o = {}; + var m = new WeakMap(), b = Symbol("b"); + m.set(o, b); + m.set(b, 1); + assert(m.get(o), b); + assert(m.get(b), 1); + b = undefined; + m = undefined; +} +f(); + +function g() { + var o = {}; + var m = new WeakMap(), a = Symbol("a"), b = Symbol("b"), c = Symbol("c"); + m.set(o, a); + m.set(a, b); + m.set(b, c); + m.set(c, 1); + a = b = c = undefined; + m = undefined; +} +g(); + +// freed when the runtime is freed +const m = new WeakMap(); +const a = Symbol("a"), b = Symbol("b"); +m.set(a, b); +m.set(b, a); +assert(m.get(a), b); +assert(m.get(b), a);