Skip to content

src: release context frame in AsyncWrap::EmitDestroy#61995

Open
Flarna wants to merge 3 commits intonodejs:mainfrom
Flarna:als-http-parser-leak
Open

src: release context frame in AsyncWrap::EmitDestroy#61995
Flarna wants to merge 3 commits intonodejs:mainfrom
Flarna:als-http-parser-leak

Conversation

@Flarna
Copy link
Member

@Flarna Flarna commented Feb 26, 2026

Release the async context frame in AsyncWrap::EmitDestroy to allow gc to collect it.

This is in special relevant for reused resources like HTTPParser otherwise they might keep ALS stores alive.

Fixes: #61882
Refs: #48528

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 26, 2026
@Flarna Flarna added the async_local_storage AsyncLocalStorage label Feb 26, 2026
Release the async context frame in AsyncWrap::EmitDestroy to allow
gc to collect it.

This is in special relevant for reused resources like HTTPParser
otherwise they might keep ALS stores alive.
@Flarna Flarna force-pushed the als-http-parser-leak branch from 391ae5a to bdd2d38 Compare February 26, 2026 00:27
@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.73%. Comparing base (488a854) to head (2500801).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/async_wrap.cc 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61995      +/-   ##
==========================================
- Coverage   89.77%   89.73%   -0.05%     
==========================================
  Files         674      676       +2     
  Lines      205705   205991     +286     
  Branches    39449    39488      +39     
==========================================
+ Hits       184670   184838     +168     
- Misses      13280    13298      +18     
- Partials     7755     7855     +100     
Files with missing lines Coverage Δ
src/async_wrap.cc 84.26% <83.33%> (-0.15%) ⬇️

... and 43 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.

@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 26, 2026
HandleScope handle_scope(env()->isolate());
USE(object()->Set(env()->context(), env()->resource_symbol(), object()));
}
context_frame_.Reset();
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but do you know how it comes this is a v8::Global to begin with and not an internal field on object()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, don't know. Maybe @Qard can chime in.

Co-authored-by: Anna Henningsen <github@addaleax.net>
@Flarna Flarna added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Feb 26, 2026
@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_local_storage AsyncLocalStorage author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AsyncContextFrame may not be released when using HTTP parsers

3 participants