-
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
AsyncLocalStorage: Cannot read property 'Symbol(kResourceStore)' #34556
Comments
Looking into the changelog I see only #34319 in this area. fyi @nodejs/async_hooks |
@johanneswuerbach that sounds like a |
I’m also wondering if this reproduces without |
@johanneswuerbach A reproducer would be really great. It would also helpful to see the complete stack. Maybe you could set |
I'll try to come up with a repro, but couldn't.
We use other services with the same plugin, so that alone seems not to be the issue. @Flarna here is the full trace (xxx is private code):
|
Thanks, that helped to get a reproducer:
|
Seems the managed and native stack get out of sync. Next call to I will not find any time during the next few day to continue with this so anyone feel free to pick it up. |
460c81d introduced a bug where the execution resource was not stored properly if we needed to call into C++ to extend the stack size. Fix that bug by always storing the resource. Refs: nodejs#34319 Fixes: nodejs#34556
Inspired by the callstack at nodejs#34556 (comment) If the wanted store is equal to the active store it's not needed to create an AsyncResource. Refs: nodejs#34556 (comment)
460c81d introduced a bug where the execution resource was not stored properly if we needed to call into C++ to extend the stack size. Fix that bug by always storing the resource. Refs: #34319 Fixes: #34556 PR-URL: #34573 Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Gus Caplan <me@gus.host>
Inspired by the callstack at #34556 (comment) If the wanted store is equal to the active store it's not needed to create an AsyncResource. Refs: #34556 (comment) PR-URL: #34616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Inspired by the callstack at #34556 (comment) If the wanted store is equal to the active store it's not needed to create an AsyncResource. Refs: #34556 (comment) PR-URL: #34616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
460c81d introduced a bug where the execution resource was not stored properly if we needed to call into C++ to extend the stack size. Fix that bug by always storing the resource. Refs: #34319 Fixes: #34556 PR-URL: #34573 Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Gus Caplan <me@gus.host>
Inspired by the callstack at #34556 (comment) If the wanted store is equal to the active store it's not needed to create an AsyncResource. Refs: #34556 (comment) PR-URL: #34616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
460c81d introduced a bug where the execution resource was not stored properly if we needed to call into C++ to extend the stack size. Fix that bug by always storing the resource. Refs: #34319 Fixes: #34556 PR-URL: #34573 Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Gus Caplan <me@gus.host>
460c81d introduced a bug where the execution resource was not stored properly if we needed to call into C++ to extend the stack size. Fix that bug by always storing the resource. Refs: #34319 Fixes: #34556 PR-URL: #34573 Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Gus Caplan <me@gus.host>
Inspired by the callstack at #34556 (comment) If the wanted store is equal to the active store it's not needed to create an AsyncResource. Refs: #34556 (comment) PR-URL: #34616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Is fix in v14.21.3? |
Also getting this error for v14.20.0 |
460c81d introduced a bug where the execution resource was not stored properly if we needed to call into C++ to extend the stack size. Fix that bug by always storing the resource. Refs: nodejs/node#34319 Fixes: nodejs/node#34556 PR-URL: nodejs/node#34573 Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Gus Caplan <me@gus.host>
What steps will reproduce the bug?
I'm not entirely sure yet, but will continue investigating. Any pointers would be appreciated.
Sadly the code itself is private, but I'll try to extract a reproduction.
How often does it reproduce? Is there a required condition?
I only see this crash reliably on every test run on one of our services, all the others seem to work fine.
The service works on v14.4.0 and v14.5.0, so this seems to be a regression introduced in v14.6.0.
What is the expected behavior?
Working AsyncLocalStorage.
What do you see instead?
An exception
Likely the result of
executionAsyncResource()
returningundefined
here:node/lib/async_hooks.js
Line 219 in 13c5a16
Additional information
AsyncLocalStorage is used using https://github.com/open-telemetry/opentelemetry-js/tree/v0.10.1 and via the
AsyncLocalStorageContextManager
https://github.com/open-telemetry/opentelemetry-js/blob/v0.10.1/packages/opentelemetry-context-async-hooks/src/AsyncLocalStorageContextManager.tsThe text was updated successfully, but these errors were encountered: