Skip to content

Fix weakmap gc #1016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions api-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,48 @@ static void two_byte_string(void)
JS_FreeRuntime(rt);
}

static void weak_map_gc_check(void)
{
static const char init_code[] =
"const map = new WeakMap(); \
function addItem() { \
const k = { \
text: 'a', \
}; \
map.set(k, {k}); \
}";
static const char test_code[] = "addItem()";

JSRuntime *rt = JS_NewRuntime();
JSContext *ctx = JS_NewContext(rt);

JSValue ret = JS_Eval(ctx, init_code, strlen(init_code), "<input>", JS_EVAL_TYPE_GLOBAL);
assert(!JS_IsException(ret));

JSValue ret_test = JS_Eval(ctx, test_code, strlen(test_code), "<input>", JS_EVAL_TYPE_GLOBAL);
assert(!JS_IsException(ret_test));
JS_RunGC(rt);
JSMemoryUsage memory_usage;
JS_ComputeMemoryUsage(rt, &memory_usage);

for (int i = 0; i < 3; i++) {
JSValue ret_test2 = JS_Eval(ctx, test_code, strlen(test_code), "<input>", JS_EVAL_TYPE_GLOBAL);
assert(!JS_IsException(ret_test2));
JS_RunGC(rt);
JSMemoryUsage memory_usage2;
JS_ComputeMemoryUsage(rt, &memory_usage2);

assert(memory_usage.memory_used_count == memory_usage2.memory_used_count);
assert(memory_usage.memory_used_size == memory_usage2.memory_used_size);
JS_FreeValue(ctx, ret_test2);
}

JS_FreeValue(ctx, ret);
JS_FreeValue(ctx, ret_test);
JS_FreeContext(ctx);
JS_FreeRuntime(rt);
}

int main(void)
{
sync_call();
Expand All @@ -278,5 +320,6 @@ int main(void)
is_array();
module_serde();
two_byte_string();
weak_map_gc_check();
return 0;
}
68 changes: 44 additions & 24 deletions quickjs.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,26 @@ typedef struct JSWeakRefRecord {
} u;
} JSWeakRefRecord;

typedef struct JSMapRecord {
int ref_count; /* used during enumeration to avoid freeing the record */
bool empty; /* true if the record is deleted */
struct JSMapState *map;
struct list_head link;
struct list_head hash_link;
JSValue key;
JSValue value;
} JSMapRecord;

typedef struct JSMapState {
bool is_weak; /* true if WeakSet/WeakMap */
struct list_head records; /* list of JSMapRecord.link */
uint32_t record_count;
struct list_head *hash_table;
uint32_t hash_size; /* must be a power of two */
uint32_t record_count_threshold; /* count at which a hash table
resize is needed */
} JSMapState;

enum {
JS_ATOM_TYPE_STRING = 1,
JS_ATOM_TYPE_GLOBAL_SYMBOL,
Expand Down Expand Up @@ -1694,8 +1714,8 @@ static JSClassShortDef const js_std_class_def[] = {
{ JS_ATOM_BigInt, js_object_data_finalizer, js_object_data_mark }, /* JS_CLASS_BIG_INT */
{ JS_ATOM_Map, js_map_finalizer, js_map_mark }, /* JS_CLASS_MAP */
{ JS_ATOM_Set, js_map_finalizer, js_map_mark }, /* JS_CLASS_SET */
{ JS_ATOM_WeakMap, js_map_finalizer, js_map_mark }, /* JS_CLASS_WEAKMAP */
{ JS_ATOM_WeakSet, js_map_finalizer, js_map_mark }, /* JS_CLASS_WEAKSET */
{ JS_ATOM_WeakMap, js_map_finalizer, NULL }, /* JS_CLASS_WEAKMAP */
{ JS_ATOM_WeakSet, js_map_finalizer, NULL }, /* JS_CLASS_WEAKSET */
{ JS_ATOM_Iterator, NULL, NULL }, /* JS_CLASS_ITERATOR */
{ JS_ATOM_IteratorHelper, js_iterator_helper_finalizer, js_iterator_helper_mark }, /* JS_CLASS_ITERATOR_HELPER */
{ JS_ATOM_IteratorWrap, js_iterator_wrap_finalizer, js_iterator_wrap_mark }, /* JS_CLASS_ITERATOR_WRAP */
Expand Down Expand Up @@ -5783,6 +5803,22 @@ void JS_MarkValue(JSRuntime *rt, JSValueConst val, JS_MarkFunc *mark_func)
}
}

static void mark_weak_map_value(JSRuntime *rt, JSWeakRefRecord *first_weak_ref, JS_MarkFunc *mark_func) {
JSWeakRefRecord *wr;
JSMapRecord *mr;
JSMapState *s;

for (wr = first_weak_ref; wr != NULL; wr = wr->next_weak_ref) {
if (wr->kind == JS_WEAK_REF_KIND_MAP) {
mr = wr->u.map_record;
s = mr->map;
assert(s->is_weak);
assert(!mr->empty); /* no iterator on WeakMap/WeakSet */
JS_MarkValue(rt, mr->value, mark_func);
}
}
}

static void mark_children(JSRuntime *rt, JSGCObjectHeader *gp,
JS_MarkFunc *mark_func)
{
Expand Down Expand Up @@ -5822,6 +5858,10 @@ static void mark_children(JSRuntime *rt, JSGCObjectHeader *gp,
prs++;
}

if (unlikely(p->first_weak_ref)) {
mark_weak_map_value(rt, p->first_weak_ref, mark_func);
}

if (p->class_id != JS_CLASS_OBJECT) {
JSClassGCMark *gc_mark;
gc_mark = rt->class_array[p->class_id].gc_mark;
Expand Down Expand Up @@ -48433,26 +48473,6 @@ static const JSCFunctionListEntry js_symbol_funcs[] = {

/* Set/Map/WeakSet/WeakMap */

typedef struct JSMapRecord {
int ref_count; /* used during enumeration to avoid freeing the record */
bool empty; /* true if the record is deleted */
struct JSMapState *map;
struct list_head link;
struct list_head hash_link;
JSValue key;
JSValue value;
} JSMapRecord;

typedef struct JSMapState {
bool is_weak; /* true if WeakSet/WeakMap */
struct list_head records; /* list of JSMapRecord.link */
uint32_t record_count;
struct list_head *hash_table;
uint32_t hash_size; /* must be a power of two */
uint32_t record_count_threshold; /* count at which a hash table
resize is needed */
} JSMapState;

#define MAGIC_SET (1 << 0)
#define MAGIC_WEAK (1 << 1)

Expand Down Expand Up @@ -49056,10 +49076,10 @@ static void js_map_mark(JSRuntime *rt, JSValueConst val,

s = p->u.map_state;
if (s) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (s) {
if (s) {
assert(!s->is_weak);

assert(!s->is_weak);
list_for_each(el, &s->records) {
mr = list_entry(el, JSMapRecord, link);
if (!s->is_weak)
JS_MarkValue(rt, mr->key, mark_func);
JS_MarkValue(rt, mr->key, mark_func);
JS_MarkValue(rt, mr->value, mark_func);
}
}
Expand Down