Fix weakmap gc#1016
Conversation
bnoordhuis
left a comment
There was a problem hiding this comment.
Can you walk me through how this fixes the bug? mr->value is visited with or without your change, right?
|
|
||
| static void weak_map_gc_check(void) | ||
| { | ||
| const char *code = |
There was a problem hiding this comment.
| const char *code = | |
| static const char code[] = |
| JSValue ret = JS_Eval(ctx, code, strlen(code), "<input>", JS_EVAL_TYPE_GLOBAL); | ||
| assert(!JS_IsException(ret)); | ||
| JS_RunGC(rt); | ||
| JSMemoryUsage memory_usage; | ||
| JS_ComputeMemoryUsage(rt, &memory_usage); | ||
| assert(memory_usage.memory_used_count < 10000); | ||
| assert(memory_usage.memory_used_size < 1024 * 1024); |
There was a problem hiding this comment.
This can be made more precise by running the script once, recording the used memory, then running it a few more times, checking each time used memory stays the same.
| if (s) { | ||
| list_for_each(el, &s->records) { | ||
| mr = list_entry(el, JSMapRecord, link); | ||
| if (!s->is_weak) | ||
| if (!s->is_weak) { | ||
| JS_MarkValue(rt, mr->key, mark_func); | ||
| JS_MarkValue(rt, mr->value, mark_func); | ||
| JS_MarkValue(rt, mr->value, mark_func); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If you're going to do this, you can change the outer if (s) { to if (s && !s->is_weak) { but that turns the function into a no-op so you might as well remove it as the mark function for JS_CLASS_WEAKMAP and JS_CLASS_WEAKSET.
Thanks, I've updated the code as you suggested. Without this change: With this change:
|
bnoordhuis
left a comment
There was a problem hiding this comment.
LGTM with one final suggestion.
| @@ -49058,8 +49078,7 @@ static void js_map_mark(JSRuntime *rt, JSValueConst val, | |||
| if (s) { | |||
There was a problem hiding this comment.
| if (s) { | |
| if (s) { | |
| assert(!s->is_weak); |
|
There seems to be a new bug in the |
I'm sorry, There are no bugs. |
|
Thanks, merged. |
The values in weak map should be mark as reachable only when it's key is reachable.
Fix #1015