avoid freeing an object structure in js_weakref_free() if it is about to be freed...

diff --git a/quickjs.c b/quickjs.c
index 09eac2ddc04828d65056cd1e078e2109f9cb5495..e6253e69256f2fd0b0c7677cbe368a5579e092cd 100644 (file)
--- a/quickjs.c
+++ b/quickjs.c
@@ -5765,6 +5765,8 @@ static void free_object(JSRuntime *rt, JSObject *p)
         /* keep the object structure in case there are weak references to it */
         if (p->weakref_count == 0) {
             js_free_rt(rt, p);
+        } else {
+            p->header.mark = 0; /* reset the mark so that the weakref can be freed */
         }
     }
 }
@@ -5850,6 +5852,7 @@ void __JS_FreeValueRT(JSRuntime *rt, JSValue v)
             if (rt->gc_phase != JS_GC_PHASE_REMOVE_CYCLES) {
                 list_del(&p->link);
                 list_add(&p->link, &rt->gc_zero_ref_count_list);
+                p->mark = 1; /* indicate that the object is about to be freed */
                 if (rt->gc_phase == JS_GC_PHASE_NONE) {
                     free_zero_refcount(rt);
                 }
@@ -46607,13 +46610,12 @@ static void js_weakref_free(JSRuntime *rt, JSValue val)
         JSObject *p = JS_VALUE_GET_OBJ(val);
         assert(p->weakref_count >= 1);
         p->weakref_count--;
-        if (p->weakref_count == 0 && p->header.ref_count == 0) {
-            if (rt->gc_phase == JS_GC_PHASE_REMOVE_CYCLES && p->header.mark) {
-                /* cannot remove now the structure if it is involved in a cycle */
-            } else {
-                /* can remove the dummy structure */
-                js_free_rt(rt, p);
-            }
+        /* 'mark' is tested to avoid freeing the object structure when
+           it is about to be freed in a cycle or in
+           free_zero_refcount() */
+        if (p->weakref_count == 0 && p->header.ref_count == 0 &&
+            p->header.mark == 0) {
+            js_free_rt(rt, p);
         }
     } else if (JS_VALUE_GET_TAG(val) == JS_TAG_SYMBOL) {
         JSString *p = JS_VALUE_GET_STRING(val);

diff --git a/tests/test_builtin.js b/tests/test_builtin.js
index 1ba59cd31a8e00a3b9dcdc2d7adce17391c19378..7ff303f8fe9adb981ca1515b09c7db47dfc2571a 100644 (file)
--- a/tests/test_builtin.js
+++ b/tests/test_builtin.js
@@ -809,6 +809,31 @@ function test_weak_map()
     /* the WeakMap should be empty here */
 }
 
+function test_weak_map_cycles()
+{
+    const weak1 = new WeakMap();
+    const weak2 = new WeakMap();
+    function createCyclicKey() {
+        const parent = {};
+        const child = {parent};
+        parent.child = child;
+        return child;
+    }
+    function testWeakMap() {
+        const cyclicKey = createCyclicKey();
+        const valueOfCyclicKey = {};
+        weak1.set(cyclicKey, valueOfCyclicKey);
+        weak2.set(valueOfCyclicKey, 1);
+    }
+    testWeakMap();
+    // Force to free cyclicKey.
+    std.gc();
+    // Here will cause sigsegv because [cyclicKey] and [valueOfCyclicKey] in [weak1] was free,
+    // but weak2's map record was not removed, and it's key refers [valueOfCyclicKey] which is free.
+    weak2.get({});
+    std.gc();
+}
+
 function test_weak_ref()
 {
     var w1, w2, o, i;
@@ -953,6 +978,7 @@ test_regexp();
 test_symbol();
 test_map();
 test_weak_map();
+test_weak_map_cycles();
 test_weak_ref();
 test_finalization_registry();
 test_generator();