Skip to content

Fix memory leak in Set.prototype.union / symmetricDifference - #1567

Merged
saghul merged 1 commit into
masterfrom
fix-set-leak
Jul 14, 2026
Merged

Fix memory leak in Set.prototype.union / symmetricDifference#1567
saghul merged 1 commit into
masterfrom
fix-set-leak

Conversation

@saghul

@saghul saghul commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Fixes: #1565

@saghul saghul mentioned this pull request Jul 14, 2026
Closed
bnoordhuis
bnoordhuis approved these changes Jul 14, 2026
View reviewed changes

@bnoordhuis bnoordhuis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but js_set_intersection has the same issue in two places; item is passed to map_add_record but not JS_FreeValue'd afterwards. There's an instance of that pattern at the bottom of js_set_symmetricDifference, too.

FWIW, I find map_add_record's semantics counterintuitive. I'd expect it to take ownership.

@saghul

saghul commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

js_set_intersection has the same issue in two places; item is passed to map_add_record but not JS_FreeValue'd afterwards. There's an instance of that pattern at the bottom of js_set_symmetricDifference, too.

Good catch, will fix!

FWIW, I find map_add_record's semantics counterintuitive. I'd expect it to take ownership.

I can reverse the logic in all callsites, if you think that's cleaner. I don't have a strong opinion here. Or I can do it in a follow-up PR.

@saghul

saghul commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the extra bugs.

I'll look into reworking map_add_record in a follow-up PR. It does have weird semantics, specially when you add weak keys into the mix...

@saghul
saghul merged commit 6d9bfbd into master Jul 14, 2026
128 checks passed
@saghul
saghul deleted the fix-set-leak branch July 14, 2026 15:17
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
HarmonybrewBot HarmonybrewBot
!15602 merge bump-quickjs-ng-0.16.0 into main
4b1137a 
quickjs-ng 0.16.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>## What's Changed
* Add new project by @qr243vbi in quickjs-ng/quickjs#1529
* Disable standalone executable support under WASM / Emscripten by @saghul in quickjs-ng/quickjs#1528
* Only export API symbols for shared builds by @dannote in quickjs-ng/quickjs#1525
* Update CI configuration to use Visual Studio 18 by @saghul in quickjs-ng/quickjs#1533
* Update test262 by @saghul in quickjs-ng/quickjs#1539
* Fix -Wmaybe-uninitialized warning in js_binary_logic_slow by @sdrsdr in quickjs-ng/quickjs#1530
* Implement Error.prototype.stack accessor proposal by @saghul in quickjs-ng/quickjs#1536
* Enforce immutability on TypedArray write paths by @saghul in quickjs-ng/quickjs#1537
* Fix ArrayBuffer immutable-method semantics by @saghul in quickjs-ng/quickjs#1538
* Implement nonextensible-applies-to-private proposal by @saghul in quickjs-ng/quickjs#1540
* Bump actions/checkout from 6 to 7 by @dependabot[bot] in quickjs-ng/quickjs#1542
* Default to release builds in Meson by @saghul in quickjs-ng/quickjs#1544
* JS_Free*: public vararg macros by @vitawrap in quickjs-ng/quickjs#1535
* Build release binaries with static mimalloc by @saghul in quickjs-ng/quickjs#1545
* Add Unicode license to libunicode-table.h by @bnoordhuis in quickjs-ng/quickjs#1547
* Port Bellard's register-based regexp engine by @saghul in quickjs-ng/quickjs#1548
* Fix build with strict C compilers on x86-32 by @trufae in quickjs-ng/quickjs#1550
* Inline mixed int/float arithmetic and fast-array reads in the interpreter by @saghul in quickjs-ng/quickjs#1549
* Add arena allocator by @saghul in quickjs-ng/quickjs#1551
* Fix `set Iterator.prototype.constructor` for non-object values by @saghul in quickjs-ng/quickjs#1553
* Fix reference leak in `Iterator.prototype.filter` by @saghul in quickjs-ng/quickjs#1561
* Fix Iterator Helper close when the source iterator has no `return` by @saghul in quickjs-ng/quickjs#1558
* Set module internal promise rejection as handled when appropriate by @xim in quickjs-ng/quickjs#1554
* Add a GC-stress CI job by @saghul in quickjs-ng/quickjs#1568
* Fix memory leak in Set.prototype.union / symmetricDifference by @saghul in quickjs-ng/quickjs#1567
* Fix use-after-free in DisposableStack disposal by @saghul in quickjs-ng/quickjs#1566
* Fix type confusion in RegExp.escape with rope strings by @saghul in quickjs-ng/quickjs#1574
* Fix use-after-free from Array `.length` grow + `push` by @saghul in quickjs-ng/quickjs#1563
* Fix reference leak in Iterator.prototype.find by @saghul in quickjs-ng/quickjs#1573
* Fix BigInt.asUintN of negatives when width is limb-aligned by @littledivy in quickjs-ng/quickjs#1575
* Fix missing module exports for array destructuring by @littledivy in quickjs-ng/quickjs#1576
* Fix double free of a detached ArrayBuffer's backing store by @littledivy in quickjs-ng/quickjs#1578
* Fix for-await-of calling return() on normal completion by @littledivy in quickjs-ng/quickjs#1577
* Await async iterator return() when a for-await loop breaks by @littledivy in quickjs-ng/quickjs#1580
* Make Array.fromAsync elements enumerable by @littledivy in quickjs-ng/quickjs#1581
* Report the correct column number for syntax errors by @littledivy in quickjs-ng/quickjs#1582
* Implement CallSite.prototype.isConstructor() by @littledivy in quickjs-ng/quickjs#1584
* Fix crash on cyclic re-export of an imported binding by @littledivy in quickjs-ng/quickjs#1583
* Name the re-exported module when a re-export is missing by @littledivy in quickjs-ng/quickjs#1586
* Report the correct column for an undefined global reference by @littledivy in quickjs-ng/quickjs#1585
* docs: add RongJS to projects by @butterflyfish in quickjs-ng/quickjs#1589
* Limit named group scopes to 256 by @saghul in quickjs-ng/quickjs#1592
* Bump actions/setup-node from 6 to 7 by @dependabot[bot] in quickjs-ng/quickjs#1593
* Harden against hash collisions by @bnoordhuis in quickjs-ng/quickjs#1590
* Add man pages by @suve in quickjs-ng/quickjs#1595
* Return null from CallSite position getters for native frames by @littledivy in quickjs-ng/quickjs#1597
* Fix subarray species constructor arguments by @littledivy in quickjs-ng/quickjs#1600
* Continue the TypedArray lastIndexOf scan after a shrink by @littledivy in quickjs-ng/quickjs#1602
* Copy bytes in increasing order in TypedArray slice by @littledivy in quickjs-ng/quickjs#1601
* Key module identity by specifier and import attributes by @littledivy in quickjs-ng/quickjs#1603
* Add JS_PromiseMarkAsHandled by @littledivy in quickjs-ng/quickjs#1604
* Do not coerce the value on a TypedArray out-of-bounds write with a foreign receiver by @littledivy in quickjs-ng/quickjs#1599
* Add JS_PromiseThen by @littledivy in quickjs-ng/quickjs#1605
* Fix UAF of a suspended coroutine referenced only via a closure by @saghul in quickjs-ng/quickjs#1569
* Fix double free of a CallSite when the backtrace array insertion fails by @saghul in quickjs-ng/quickjs#1614
* Fix use-after-free when creating the promise resolving functions OOMs by @saghul in quickjs-ng/quickjs#1613
* Fix leak in JSON.stringify() when space is a rope by @saghul in quickjs-ng/quickjs#1612
* Fix integer overflow in the TypedArray sort index array allocation by @saghul in quickjs-ng/quickjs#1611
* Fix reference leak in Atomics.store() on an invalidated buffer by @saghul in quickjs-ng/quickjs#1610
* Fix integer overflow in the Proxy ownKeys() property enumeration by @saghul in quickjs-ng/quickjs#1609
* Fix integer multiplication result cast to unsigned long in js_realloc_array() by @Nidrax in quickjs-ng/quickjs#1608
* Drop js_realloc2 and its allocation-slack feedback by @saghul in quickjs-ng/quickjs#1616
* lre: Do not patch a jump offset that could not be emitted by @saghul in quickjs-ng/quickjs#1615
* Reject out-of-range limits in Iterator.prototype.take/drop by @saghul in quickjs-ng/quickjs#1618
* Implement the Iterator.prototype.join() proposal by @saghul in quickjs-ng/quickjs#1620
* Implement the iterator includes proposal by @saghul in quickjs-ng/quickjs#1619
* Support resizable externally managed ArrayBuffers by @saghul in quickjs-ng/quickjs#1594
* Implement the iterator chunking proposal by @saghul in quickjs-ng/quickjs#1621
* Iterator helper fixes by @saghul in quickjs-ng/quickjs#1622
* Iterator.contact fixes by @saghul in quickjs-ng/quickjs#1623

## New Contributors
* @dannote made their first contribution in quickjs-ng/quickjs#1525
* @sdrsdr made their first contribution in quickjs-ng/quickjs#1530
* @vitawrap made their first contribution in quickjs-ng/quickjs#1535
* @xim made their first contribution in quickjs-ng/quickjs#1554
* @butterflyfish made their first contribution in quickjs-ng/quickjs#1589
* @suve made their first contribution in quickjs-ng/quickjs#1595
* @Nidrax made their first contribution in quickjs-ng/quickjs#1608

**Full Changelog**: https://github.com/quickjs-ng/quickjs/compare/v0.15.1...v0.16.0</pre>
  <p>View the full release notes at <a href="https://github.com/quickjs-ng/quickjs/releases/tag/v0.16.0">https://github.com/quickjs-ng/quickjs/releases/tag/v0.16.0</a>.</p>
</details>
<hr>

See merge request: Harmonybrew/homebrew-core!15602
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

2 participants

@saghul @bnoordhuis