Skip to content

[v25.x backport] src: convert context_frame field in AsyncWrap to internal field#62357

Open
Flarna wants to merge 1 commit intonodejs:v25.x-stagingfrom
Flarna:v25-bp-62103
Open

[v25.x backport] src: convert context_frame field in AsyncWrap to internal field#62357
Flarna wants to merge 1 commit intonodejs:v25.x-stagingfrom
Flarna:v25-bp-62103

Conversation

@Flarna
Copy link
Member

@Flarna Flarna commented Mar 20, 2026

Backport the second (7dd5ca8 / 6ad5f7c) commit of #62103 as it failed in 25.8.1 release. The other two commits were included in 25.8.1

Refs: #62103

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: nodejs#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 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. v25.x Issues that can be reproduced on v25.x or PRs targeting the v25.x-staging branch. labels Mar 20, 2026
@Flarna Flarna marked this pull request as draft March 20, 2026 23:58
@Flarna Flarna marked this pull request as ready for review March 21, 2026 22:44
@Flarna Flarna marked this pull request as draft March 22, 2026 00:32
@Flarna Flarna marked this pull request as ready for review March 22, 2026 21:40
@Flarna
Copy link
Member Author

Flarna commented Mar 22, 2026

Finally I was able to reproduce the failed test locally by using clang (19.1.1) like in Github CI instead gcc (13.3.0).
Different behavior between clang and gcc smells fishy.
As far as I know main uses also clang but this test is green on main which is strange.
I found also that this test fails only for release builds, not debug builds which is again something I do not like.

I'm not that a big expert on V8 internals/snapshots.

@addaleax @joyeecheung any hints welcome how to find out why v25 shows these instability in snapshot creation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. v25.x Issues that can be reproduced on v25.x or PRs targeting the v25.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants