Skip to content

src: convert context_frame field in AsyncWrap to internal field#62103

Closed
addaleax wants to merge 3 commits intonodejs:mainfrom
addaleax:async-wrap-async-context-frame-gc
Closed

src: convert context_frame field in AsyncWrap to internal field#62103
addaleax wants to merge 3 commits intonodejs:mainfrom
addaleax:async-wrap-async-context-frame-gc

Conversation

@addaleax
Copy link
Member

@addaleax addaleax commented Mar 4, 2026

Refs: #61995 (comment)

src: make AsyncWrap subclass internal field counts explicit

This helps ensure that we already set the correct number of
internal fields when creating objects, even if the number of
internal fields of e.g. AsyncWrap changes over time.

src: convert context_frame field in AsyncWrap to internal field

Using an internal field instead of a v8::Global<> removes
an unnecessary memory leak footgun.

This includes a test that demonstrates the issue, albeit
using internal APIs.

It's worth noting that if this PR is not accepted, we'd still
be missing memory tracking for the context_frame_ field,
and we'd need to add it through our memory tracking API.

src: expose async context frame debugging helper to JS

This was invaluable for debugging the previous change,
so I'm suggesting we add it regardless of it being a
debugging-only API.

addaleax added 3 commits March 4, 2026 16:55
@addaleax
src: make AsyncWrap subclass internal field counts explicit
13adffc 
This helps ensure that we already set the correct number of
internal fields when creating objects, even if the number of
internal fields of e.g. AsyncWrap changes over time.
@addaleax
src: convert context_frame field in AsyncWrap to internal field
9f06ed5 
Using an internal field instead of a `v8::Global<>` removes
an unnecessary memory leak footgun.

This includes a test that demonstrates the issue, albeit
using internal APIs.

It's worth noting that if this PR is not accepted, we'd still
be missing memory tracking for the `context_frame_` field,
and we'd need to add it through our memory tracking API.
@addaleax
src: expose async context frame debugging helper to JS
8a82488 
This was invaluable for debugging the previous change,
so I'm suggesting we add it regardless of it being a
debugging-only API.
@addaleax addaleax requested review from Flarna and Qard March 4, 2026 16:22
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 4, 2026

Review requested:

  • @nodejs/crypto
  • @nodejs/http2
  • @nodejs/net
  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Mar 4, 2026
@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Mar 4, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 4, 2026
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@codecov
Copy link

codecov bot commented Mar 4, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 92.98246% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.64%. Comparing base (afc30b7) to head (8a82488).
⚠️ Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
src/async_wrap.cc 71.42% 4 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #62103   +/-   ##
=======================================
  Coverage   89.64%   89.64%           
=======================================
  Files         676      676           
  Lines      206326   206343   +17     
  Branches    39528    39532    +4     
=======================================
+ Hits       184960   184985   +25     
- Misses      13476    13483    +7     
+ Partials     7890     7875   -15
Files with missing lines Coverage Δ
src/async_wrap-inl.h 95.12% <100.00%> (+2.81%) ⬆️
src/base_object.cc 84.46% <100.00%> (+0.15%) ⬆️
src/base_object.h 100.00% <ø> (ø)
src/cares_wrap.cc 61.64% <100.00%> (-0.09%) ⬇️
src/crypto/crypto_keys.cc 73.75% <ø> (ø)
src/crypto/crypto_sig.cc 70.34% <100.00%> (ø)
src/crypto/crypto_tls.cc 77.85% <100.00%> (ø)
src/crypto/crypto_tls.h 86.66% <ø> (ø)
src/crypto/crypto_util.h 74.79% <ø> (ø)
src/crypto/crypto_x509.cc 72.44% <ø> (ø)
... and 20 more

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 4, 2026

CI: https://ci.nodejs.org/job/node-test-pull-request/71571/

@Flarna Flarna added dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. async_local_storage AsyncLocalStorage labels Mar 4, 2026
@addaleax addaleax added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 4, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 6, 2026
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 6, 2026

Landed in 1947018...50fe010

nodejs-github-bot pushed a commit that referenced this pull request Mar 6, 2026
@addaleax @nodejs-github-bot
src: make AsyncWrap subclass internal field counts explicit
b872cc2 
This helps ensure that we already set the correct number of
internal fields when creating objects, even if the number of
internal fields of e.g. AsyncWrap changes over time.

PR-URL: #62103
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Mar 6, 2026
@addaleax @nodejs-github-bot
src: convert context_frame field in AsyncWrap to internal field
6ad5f7c 
Using an internal field instead of a `v8::Global<>` removes
an unnecessary memory leak footgun.

This includes a test that demonstrates the issue, albeit
using internal APIs.

It's worth noting that if this PR is not accepted, we'd still
be missing memory tracking for the `context_frame_` field,
and we'd need to add it through our memory tracking API.

PR-URL: #62103
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Mar 6, 2026
@addaleax @nodejs-github-bot
src: expose async context frame debugging helper to JS
50fe010 
This was invaluable for debugging the previous change,
so I'm suggesting we add it regardless of it being a
debugging-only API.

PR-URL: #62103
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Mar 10, 2026
@addaleax @aduh95
src: make AsyncWrap subclass internal field counts explicit
0257e80 
This helps ensure that we already set the correct number of
internal fields when creating objects, even if the number of
internal fields of e.g. AsyncWrap changes over time.

PR-URL: #62103
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Mar 10, 2026
@addaleax @aduh95
src: convert context_frame field in AsyncWrap to internal field
7dd5ca8 
Using an internal field instead of a `v8::Global<>` removes
an unnecessary memory leak footgun.

This includes a test that demonstrates the issue, albeit
using internal APIs.

It's worth noting that if this PR is not accepted, we'd still
be missing memory tracking for the `context_frame_` field,
and we'd need to add it through our memory tracking API.

PR-URL: #62103
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Mar 10, 2026
@addaleax @aduh95
src: expose async context frame debugging helper to JS
8f3b126 
This was invaluable for debugging the previous change,
so I'm suggesting we add it regardless of it being a
debugging-only API.

PR-URL: #62103
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@github-actions github-actions bot mentioned this pull request Mar 10, 2026
Merged
aduh95 pushed a commit that referenced this pull request Mar 10, 2026
@addaleax @aduh95
src: expose async context frame debugging helper to JS
19efe60 
This was invaluable for debugging the previous change,
so I'm suggesting we add it regardless of it being a
debugging-only API.

PR-URL: #62103
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@aduh95 aduh95 added the backport-requested-v25.x PRs awaiting manual backport to the v25.x-staging branch. label Mar 10, 2026
@aduh95
Copy link
Contributor

aduh95 commented Mar 10, 2026 •
edited
Loading

7dd5ca8 / 6ad5f7c is making test-snapshot-reproducible fail on v25.x-staging

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Mar 12, 2026
build(deps): update node.js to v25.8.1
b794892 
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [node](https://nodejs.org) ([source](https://github.com/nodejs/node)) | patch | `25.8.0` → `25.8.1` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>nodejs/node (node)</summary>

### [`v25.8.1`](https://github.com/nodejs/node/releases/tag/v25.8.1): 2026-03-11, Version 25.8.1 (Current), @&#8203;aduh95

[Compare Source](nodejs/node@v25.8.0...v25.8.1)

##### Notable Changes

- \[[`ea87eea71a`](nodejs/node@ea87eea71a)] - **module**: fix extensionless CJS files in `"type": "module"` packages (Matteo Collina) [#&#8203;62083](nodejs/node#62083)

##### Commits

- \[[`bab750d1b3`](nodejs/node@bab750d1b3)] - **build**: do not depend on V8 deps on `--without-bundled-v8` builds (Antoine du Hamel) [#&#8203;62033](nodejs/node#62033)
- \[[`b26d1c7fcb`](nodejs/node@b26d1c7fcb)] - **crypto**: make --use-system-ca per-env rather than per-process (Aditi) [#&#8203;60678](nodejs/node#60678)
- \[[`e362635abf`](nodejs/node@e362635abf)] - **crypto**: add missing AES dictionaries (Filip Skokan) [#&#8203;62099](nodejs/node#62099)
- \[[`6f975db8af`](nodejs/node@6f975db8af)] - **crypto**: fix importKey required argument count check (Filip Skokan) [#&#8203;62099](nodejs/node#62099)
- \[[`3beaf9c5fc`](nodejs/node@3beaf9c5fc)] - **deps**: update amaro to 1.1.8 (Node.js GitHub Bot) [#&#8203;62151](nodejs/node#62151)
- \[[`53afb0edd8`](nodejs/node@53afb0edd8)] - **deps**: update sqlite to 3.52.0 (Node.js GitHub Bot) [#&#8203;62150](nodejs/node#62150)
- \[[`a13ed052a1`](nodejs/node@a13ed052a1)] - **deps**: update merve to 1.2.0 (Node.js GitHub Bot) [#&#8203;62149](nodejs/node#62149)
- \[[`2c850577b7`](nodejs/node@2c850577b7)] - **deps**: patch resb crate (Richard Lau) [#&#8203;62138](nodejs/node#62138)
- \[[`37862a6728`](nodejs/node@37862a6728)] - **deps**: V8: cherry-pick [`aa0b288`](nodejs/node@aa0b288f87cc) (Richard Lau) [#&#8203;62136](nodejs/node#62136)
- \[[`09191ad8b4`](nodejs/node@09191ad8b4)] - **deps**: update ada to 3.4.3 (Node.js GitHub Bot) [#&#8203;62049](nodejs/node#62049)
- \[[`8d63a178fd`](nodejs/node@8d63a178fd)] - **doc**: copyedit `addons.md` (Antoine du Hamel) [#&#8203;62071](nodejs/node#62071)
- \[[`83719ffb64`](nodejs/node@83719ffb64)] - **doc**: correct `util.convertProcessSignalToExitCode` validation behavior (René) [#&#8203;62134](nodejs/node#62134)
- \[[`eeee7c7fb1`](nodejs/node@eeee7c7fb1)] - **doc**: add efekrskl as triager (Efe) [#&#8203;61876](nodejs/node#61876)
- \[[`db150b2e69`](nodejs/node@db150b2e69)] - **doc**: fix markdown for `expectFailure` values (Jacob Smith) [#&#8203;62100](nodejs/node#62100)
- \[[`d55a441e60`](nodejs/node@d55a441e60)] - **doc**: add title to index (Aviv Keller) [#&#8203;62046](nodejs/node#62046)
- \[[`cc46204b48`](nodejs/node@cc46204b48)] - **doc**: include url.resolve() in DEP0169 application deprecation (Mike McCready) [#&#8203;62002](nodejs/node#62002)
- \[[`1d91a7261e`](nodejs/node@1d91a7261e)] - **doc,module**: add missing doc for syncHooks.deregister() (Joyee Cheung) [#&#8203;61959](nodejs/node#61959)
- \[[`5198573bee`](nodejs/node@5198573bee)] - **http**: fix use-after-free when freeParser is called during llhttp\_execute (Gerhard Stöbich) [#&#8203;62095](nodejs/node#62095)
- \[[`f8793f80df`](nodejs/node@f8793f80df)] - **lib**: fix source map url parse in dynamic imports (Chengzhong Wu) [#&#8203;61990](nodejs/node#61990)
- \[[`5439d0e0cf`](nodejs/node@5439d0e0cf)] - **meta**: bump actions/download-artifact from 7.0.0 to 8.0.0 (dependabot\[bot]) [#&#8203;62063](nodejs/node#62063)
- \[[`27fd21943a`](nodejs/node@27fd21943a)] - **meta**: bump actions/upload-artifact from 6.0.0 to 7.0.0 (dependabot\[bot]) [#&#8203;62062](nodejs/node#62062)
- \[[`5b266f3295`](nodejs/node@5b266f3295)] - **meta**: bump step-security/harden-runner from 2.14.2 to 2.15.0 (dependabot\[bot]) [#&#8203;62064](nodejs/node#62064)
- \[[`ea87eea71a`](nodejs/node@ea87eea71a)] - **module**: fix extensionless CJS files in `"type": "module"` packages (Matteo Collina) [#&#8203;62083](nodejs/node#62083)
- \[[`851228cd60`](nodejs/node@851228cd60)] - **sqlite**: handle stmt invalidation (Guilherme Araújo) [#&#8203;61877](nodejs/node#61877)
- \[[`19efe60548`](nodejs/node@19efe60548)] - **src**: expose async context frame debugging helper to JS (Anna Henningsen) [#&#8203;62103](nodejs/node#62103)
- \[[`0257e8072f`](nodejs/node@0257e8072f)] - **src**: make AsyncWrap subclass internal field counts explicit (Anna Henningsen) [#&#8203;62103](nodejs/node#62103)
- \[[`975dafbe3b`](nodejs/node@975dafbe3b)] - **src**: release context frame in AsyncWrap::EmitDestroy (Gerhard Stöbich) [#&#8203;61995](nodejs/node#61995)
- \[[`f2c08c7888`](nodejs/node@f2c08c7888)] - **src**: use validate\_ascii\_with\_errors instead of validate\_ascii (Сковорода Никита Андреевич) [#&#8203;61122](nodejs/node#61122)
- \[[`0278461d83`](nodejs/node@0278461d83)] - **stream**: optimize webstreams pipeTo (Mattias Buelens) [#&#8203;62079](nodejs/node#62079)
- \[[`4d62e95bfa`](nodejs/node@4d62e95bfa)] - **stream**: fix brotli error handling in web compression streams (Filip Skokan) [#&#8203;62107](nodejs/node#62107)
- \[[`4bdcaf2865`](nodejs/node@4bdcaf2865)] - **stream**: improve Web Compression spec compliance (Filip Skokan) [#&#8203;62107](nodejs/node#62107)
- \[[`a5b1be2045`](nodejs/node@a5b1be2045)] - **stream**: fix UTF-8 character corruption in fast-utf8-stream (Matteo Collina) [#&#8203;61745](nodejs/node#61745)
- \[[`5632446c4e`](nodejs/node@5632446c4e)] - **stream**: fix TransformStream race on cancel with pending write (Marco) [#&#8203;62040](nodejs/node#62040)
- \[[`f90fa9cd1a`](nodejs/node@f90fa9cd1a)] - **stream**: accept ArrayBuffer in CompressionStream and DecompressionStream (조수민) [#&#8203;61913](nodejs/node#61913)
- \[[`00319eaa3a`](nodejs/node@00319eaa3a)] - **test**: update WPT for url to [`c928b19`](nodejs/node@c928b19ab0) (Node.js GitHub Bot) [#&#8203;62148](nodejs/node#62148)
- \[[`456abc7d20`](nodejs/node@456abc7d20)] - **test**: update WPT for WebCryptoAPI to [`c9e9558`](nodejs/node@c9e955840a) (Node.js GitHub Bot) [#&#8203;62147](nodejs/node#62147)
- \[[`82770cb7d3`](nodejs/node@82770cb7d3)] - **test**: improve WPT report runner (Filip Skokan) [#&#8203;62107](nodejs/node#62107)
- \[[`cfc847d233`](nodejs/node@cfc847d233)] - **test**: update WPT compression to [`ae05f5c`](nodejs/node@ae05f5cb53) (Filip Skokan) [#&#8203;62107](nodejs/node#62107)
- \[[`80f78f2737`](nodejs/node@80f78f2737)] - **test**: update WPT for WebCryptoAPI to [`42e4732`](nodejs/node@42e47329fd) (Node.js GitHub Bot) [#&#8203;62048](nodejs/node#62048)
- \[[`8048e0508c`](nodejs/node@8048e0508c)] - **test**: fix skipping behavior for `test-runner-run-files-undefined` (Antoine du Hamel) [#&#8203;62026](nodejs/node#62026)
- \[[`699a6214c6`](nodejs/node@699a6214c6)] - **tools**: revert timezone update GHA workflow to ubuntu-latest (Richard Lau) [#&#8203;62140](nodejs/node#62140)
- \[[`1a453b550c`](nodejs/node@1a453b550c)] - **tools**: improve error handling in test426 update script (Rich Trott) [#&#8203;62121](nodejs/node#62121)
- \[[`710dde5ee2`](nodejs/node@710dde5ee2)] - **tools**: fix `--node-builtin-modules-path` value in `shell.nix` (Antoine du Hamel) [#&#8203;62102](nodejs/node#62102)
- \[[`dcb1cbb21f`](nodejs/node@dcb1cbb21f)] - **tools**: bump the eslint group across 1 directory with 2 updates (dependabot\[bot]) [#&#8203;62092](nodejs/node#62092)
- \[[`7d0b758583`](nodejs/node@7d0b758583)] - **tools**: fix daily wpt workflow nighly release version lookup (Filip Skokan) [#&#8203;62076](nodejs/node#62076)
- \[[`3e8c816f2e`](nodejs/node@3e8c816f2e)] - **tools**: fix example in release proposal linter (Richard Lau) [#&#8203;62074](nodejs/node#62074)
- \[[`772d3d270d`](nodejs/node@772d3d270d)] - **tools**: bump minimatch from 3.1.3 to 3.1.5 in /tools/clang-format (dependabot\[bot]) [#&#8203;62013](nodejs/node#62013)
- \[[`92f3b42672`](nodejs/node@92f3b42672)] - **tools**: bump eslint to v10, babel to v8.0.0-rc.2 (Huáng Jùnliàng) [#&#8203;61905](nodejs/node#61905)
- \[[`deead95ec5`](nodejs/node@deead95ec5)] - **url**: suppress warnings from url.format/url.resolve inside node\_modules (René) [#&#8203;62005](nodejs/node#62005)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42MS43IiwidXBkYXRlZEluVmVyIjoiNDMuNjEuNyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6OnBhdGNoIl19-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Qard Qard Qard approved these changes

@cjihrig cjihrig cjihrig approved these changes

@legendecas legendecas legendecas approved these changes

@Flarna Flarna Flarna approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Assignees

No one assigned

Labels

async_local_storage AsyncLocalStorage author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v25.x PRs awaiting manual backport to the v25.x-staging branch. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@addaleax @nodejs-github-bot @aduh95 @Qard @cjihrig @legendecas @Flarna @RafaelGSS