Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

async_hooks: add an InactiveAsyncContextFrame class #54510

Merged
merged 1 commit into from
Aug 30, 2024
Merged

async_hooks: add an InactiveAsyncContextFrame class #54510

merged 1 commit into from
Aug 30, 2024

Conversation

bengl
Copy link
Member

@bengl bengl commented Aug 22, 2024

This gives a class prototype for AsyncContextFrame that contains the required methods, so that when we swap the prototype, ActiveAsyncContextFrame methods are used instead. Previously, the methods were defined in AsyncContextFrame, so swapping the prototype didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: #54503

@bengl
async_hooks: add an InactiveAsyncContextFrame class
d3608a0 
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: nodejs#54503
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 22, 2024
@bengl bengl added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 22, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 22, 2024
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@codecov Codecov
Copy link

codecov bot commented Aug 22, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.35%. Comparing base (e70bd47) to head (d3608a0).
Report is 345 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54510      +/-   ##
==========================================
+ Coverage   87.34%   87.35%   +0.01%     
==========================================
  Files         649      649              
  Lines      182524   182526       +2     
  Branches    35026    35033       +7     
==========================================
+ Hits       159420   159453      +33     
+ Misses      16373    16345      -28     
+ Partials     6731     6728       -3
Files with missing lines Coverage Δ
lib/internal/async_context_frame.js 97.36% <100.00%> (+17.63%) ⬆️

... and 31 files with indirect coverage changes

@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@RedYetiDev RedYetiDev added the async_hooks Issues and PRs related to the async hooks subsystem. label Aug 23, 2024
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@Trott
Copy link
Member

Trott commented Aug 28, 2024

I'm not sure why the ARM tests fail after 30 seconds with this PR. I'm guessing it's nothing to do wtih this PR, though. But still, we should probably investigate.....

@Qard
Copy link
Member

Qard commented Aug 28, 2024

Seems to have some very strange git-related failures. https://ci.nodejs.org/job/node-test-binary-armv7l/RUN_SUBSET=js,nodes=ubuntu2204-armv7l/13225/console 🤔

@Flarna Flarna added the async_local_storage AsyncLocalStorage label Aug 29, 2024
@Flarna Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2024
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 30, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 30, 2024
@nodejs-github-bot nodejs-github-bot merged commit ef6b9ff into nodejs:main Aug 30, 2024
68 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in ef6b9ff

sendoru pushed a commit to sendoru/node that referenced this pull request Sep 1, 2024
@bengl @sendoru
async_hooks: add an InactiveAsyncContextFrame class
3a96ec5 
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: nodejs#54503
PR-URL: nodejs#54510
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
aduh95 pushed a commit that referenced this pull request Sep 12, 2024
@bengl @aduh95
async_hooks: add an InactiveAsyncContextFrame class
027b0ff 
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: #54503
PR-URL: #54510
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@RafaelGSS RafaelGSS mentioned this pull request Sep 16, 2024
Merged
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 22, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@bengl @louwers
async_hooks: add an InactiveAsyncContextFrame class
f3e5bb0 
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: nodejs#54503
PR-URL: nodejs#54510
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
@bengl @tpoisseau
async_hooks: add an InactiveAsyncContextFrame class
df4b7ba 
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: nodejs#54503
PR-URL: nodejs#54510
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VoltrexKeyva VoltrexKeyva VoltrexKeyva approved these changes

@Flarna Flarna Flarna approved these changes

@anonrig anonrig anonrig approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

@Qard Qard Qard approved these changes

Assignees
No one assigned
Labels
async_hooks Issues and PRs related to the async hooks subsystem. async_local_storage AsyncLocalStorage dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. 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.

TypeError: Method Map.prototype.set called on incompatible receiver #<AsyncContextFrame>
10 participants
@bengl @nodejs-github-bot @Trott @Qard @jasnell @anonrig @Flarna @VoltrexKeyva @targos @RedYetiDev