Skip to content

memory leak: Set.prototype.union / symmetricDifference double-incref object elements (redundant js_dup into map_add_record) #1565

Closed

Description

@xmzyshypnc

Summary

Set.prototype.union (js_set_union, quickjs.c:53898) and Set.prototype.symmetricDifference (js_set_symmetricDifference, quickjs.c:53815) leak every object element of the receiver set. Both copy the receiver's records into an intermediate set with map_add_record(ctx, t, js_dup(mr->key)), but map_add_record already dups the key internally (mr->key = js_dup(key)), so the extra js_dup over‑increments the key's refcount by one. Primitive keys (numbers, etc.) are not reference‑counted so nothing is observable, but object keys are leaked — they are never freed.

Environment

quickjs-ng master d950d55 (2026-07-13)
Build : clang-21, cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS_RELEASE="-O2 -g"  (asserts enabled)

PoC

new Set([{}]).union(new Set([9]));                 // leaks the {} element
new Set([{}]).symmetricDifference(new Set([9]));   // same

Deterministic (20/20). new Set([1]).union(...) (a primitive element) does not leak, which is the tell: only reference‑counted (object) elements are affected.

Detection

quickjs.c:2682: void JS_FreeRuntime(JSRuntime *): Assertion `list_empty(&rt->gc_obj_list)' failed.

The over‑incremented object survives to runtime teardown. In a long‑running process each union/symmetricDifference over a set that contains objects leaks those objects — an unbounded memory leak.

Root cause

map_add_record (quickjs.c:52742) takes a borrowed key and dups it into the record:

    } else {
        mr->key = js_dup(key);      // map_add_record already owns a fresh ref
    }

js_map_set and friends therefore call it without dup'ing. But js_set_union / js_set_symmetricDifference pass an already‑dup'd key:

        mr = map_add_record(ctx, t, js_dup(mr->key));   // double incref -> leak of the object key

Suggested fix

Drop the redundant js_dup at both call sites (quickjs.c:53815 and :53898):

-        mr = map_add_record(ctx, t, js_dup(mr->key));
+        mr = map_add_record(ctx, t, mr->key);

Tested: with this change both PoCs no longer leak (0/20, was 20/20).

Credit

xmzyshypnc(@xmzyshypnc) and Yanjie Zhao(@carol233) and Yiyang Liu(@lyyffee)

Activity

  1. saghul commented on Jul 14, 2026

    @saghul
    Contributor

    Thanks for your he report! Would you like to send a PR?

  2. saghul commented on Jul 14, 2026

    @saghul
    Contributor

    Fix in #1567

  3. xmzyshypnc commented on Jul 14, 2026

    @xmzyshypnc
    Author

    Thanks for your he report! Would you like to send a PR?

    Sorry I just saw your response. Thanks for your quick fix!

  4. added 2 commits that reference this issue on Jul 14, 2026
    6d051d5
    6d9bfbd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions