Repository navigation
Conversation
js_map_finalizer() walked the records of a WeakMap and, for each one, removed its weak reference and then freed its value. When the value is the last reference to a symbol, freeing it runs reset_weak_ref() on the symbol, which unlinks and frees every record of the same map that uses the symbol as a key. If that record was the next one in the list, the loop went on with freed memory: a segfault, a heap-use-after-free under ASan, or the `wr != NULL` assertion in delete_map_weak_ref(). Remove the weak references of all records first and free the values in a second pass, like reset_weak_ref() does, so that freeing a value can no longer reach the records of the map being finalized. Map and Set are unchanged. Fixes: quickjs-ng#1835
saghul
approved these changes
Oct 24, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1835.
js_map_finalizer()walks the records of a WeakMap and, for each one, removes its weak reference and then frees its value. When the value is the last reference to a symbol, freeing it runsreset_weak_ref()on that symbol, which unlinks and frees every record of the same map that uses the symbol as a key. If that record is the onelist_for_each_safe()saved as the next element, the loop continues on freed memory. Release builds segfault, builds with asserts stop onwr != NULLindelete_map_weak_ref(), and ASan reports a heap-use-after-free injs_map_finalizer().This change removes the weak references of all records in a first pass and frees the values in a second pass, the same split that
js_finrec_finalizer()andreset_weak_ref()already use. Freeing a value then can no longer reach a record of the map being finalized. Map and Set are unchanged.tests/bug1835.jscovers both reproductions from the issue: the map freed when the runtime is freed, and the map freed when a local variable is cleared. It also covers a chain of three symbols. Without the fix, the test aborts on the assertion in a Debug build and fails with heap-use-after-free under ASan. With the fix, the test passes in Debug, ASan+UBSan and GC stress builds. The rest ofmake test, and test262 under ASan+UBSan, show no new failures.