Skip to content

async_hooks: ensure AsyncLocalStore instances work isolated - #58149

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
Flarna:flarna/als-isolated
May 6, 2025
Merged

async_hooks: ensure AsyncLocalStore instances work isolated#58149
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
Flarna:flarna/als-isolated

Conversation

@Flarna

Copy link
Copy Markdown
Member

Avoid that one AsyncLocalStore instance changes the state of another AsyncLocalStore instance by restoring only the owned store instead the complete AsyncContextFrame.

Avoid that one AsyncLocalStore instance changes the state of another
AsyncLocalStore instance by restoring only the owned store instead
the complete AsyncContextFrame.
@nodejs-github-botnodejs-github-bot added async_local_storage AsyncLocalStorage needs-ci PRs that need a full CI run. labels May 3, 2025
Comment threadtest/async-hooks/test-async-local-storage-gcable.js Outdated
@codecov

codecovBot commented May 3, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.17%. Comparing base (92102c0) to head (7f4ed98).
Report is 28 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #58149 +/- ##
==========================================
- Coverage 90.18% 90.17% -0.01% 
==========================================
Files 630 630 Lines 186473 186507 +34 Branches 36612 36618 +6 ==========================================
+ Hits 168169 168183 +14 - Misses 11116 11121 +5 - Partials 7188 7203 +15 
Files with missing linesCoverage Δ
...nternal/async_local_storage/async_context_frame.js100.00% <100.00%> (ø)

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

@legendecaslegendecas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for spotting this! LGTM

Comment threadtest/parallel/test-async-local-storage-isolation.js
@FlarnaFlarna added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v20.x labels May 4, 2025
Comment threadlib/internal/async_local_storage/async_context_frame.js
@FlarnaFlarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 5, 2025
@legendecaslegendecas added the request-ci Add this label to start a Jenkins CI on a PR. label May 5, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 5, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

@Flarna
Flarna requested a review from QardMay 5, 2025 16:44
Qard
Qard approved these changes May 5, 2025
@FlarnaFlarna added the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2025
@nodejs-github-bot
nodejs-github-bot merged commit a0d458e into nodejs:mainMay 6, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in a0d458e

@Flarna
Flarna deleted the flarna/als-isolated branch May 6, 2025 18:04
targos pushed a commit that referenced this pull request May 16, 2025
Avoid that one AsyncLocalStore instance changes the state of another
AsyncLocalStore instance by restoring only the owned store instead
the complete AsyncContextFrame.
PR-URL: #58149
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 pushed a commit that referenced this pull request Jun 10, 2025
Avoid that one AsyncLocalStore instance changes the state of another
AsyncLocalStore instance by restoring only the owned store instead
the complete AsyncContextFrame.
PR-URL: #58149
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
mcollina pushed a commit to mcollina/node that referenced this pull request Dec 12, 2025
Avoid that one AsyncLocalStore instance changes the state of another
AsyncLocalStore instance by restoring only the owned store instead
the complete AsyncContextFrame.
PR-URL: nodejs#58149
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_local_storageAsyncLocalStorageauthor readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Flarna@nodejs-github-bot@Qard@anonrig@legendecas@BridgeAR