-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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.
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
async_hooks: fix context loss after nested calls to AsyncLocalStorage #32085
async_hooks: fix context loss after nested calls to AsyncLocalStorage #32085
Conversation
3caff2f
to
4c28a3d
Compare
@puzpuzpuz I am afraid there is a curse on your PRs and CI 😝. I will resume this build 😉 |
I knew it. Sounds like the most logical reason for these permanently flaky builds. 🤣 Thanks for restarting it. |
Looks like the second attempt was successful. 🥳 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR-URL: #32085 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Landed in 86ab4ee |
PR-URL: #32085 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Blocked on #26540. |
v12 backport: #32318 |
PR-URL: nodejs#32085 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: nodejs#32085 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: #32085 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Fixes the following issues within
AsyncLocalStorage
API:run*
were erasing the outer store, leading to context loss.test-async-local-storage-nested.js
was updated to validate this fix, as its previous contents were a partial duplicate oftest-async-local-storage-no-mix-contexts.js
exit*
were switchingenabled
field totrue
without checking current value. This leads to incorrect behavior of furthergetStore
andrun*
calls.These issues were discussed in #31950, but as that PR involves significant changes in implementation and will require some time to settle down.
cc @vdeturckheim @Qard @Flarna
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes