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.

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

Node 16.2 regression, crashing with node::InternalCallbackScope::Close() error #38815

Closed
gaggle opened this issue May 26, 2021 · 5 comments
Closed
Labels
async_hooks Issues and PRs related to the async hooks subsystem.

Comments

@gaggle
Copy link

gaggle commented May 26, 2021

Upgrading from Node 16.1 to 16.2 causes Node to crash, and I don't know how to debug the problem further. This is related to promises and maybe async_hooks or cls-hooked library, but I'm not knowledgeable enough to understand how or why it actually happens.

  • Version: v16.2.0
  • Platform: Darwin 20.4.0 Darwin Kernel Version 20.4.0: Thu Apr 22 21:46:47 PDT 2021; root:xnu-7195.101.2~1/RELEASE_X86_64 x86_64
  • Subsystem:

What steps will reproduce the bug?

I have cooked up a minimal repo here: https://github.com/heap-dk/node-16.2-crash/

The problem manifests itself somehow in relation to promises and async hooks... I think! The repo's readme shows the problem, the code is as minimal as I could make it to showcase the problem, and the commands to replicate the issue are simple. I hope that's good enough to be of interest to you.

The repo has dependencies on dd-trace and cls-hooked, but I don't think the problem is unique to those dependencies, rather I think it's related to their underlying use of async_hooks and somehow it results in a crash... 👀 But I'm guessing on those details so I very much hope someone with more knowledge about these systems can see the deeper pattern!

How often does it reproduce? Is there a required condition?

100% crash.

What is the expected behavior?

I didn't expect a Node patch bump to introduce a breaking behavior like this.

What do you see instead?

node[90090]: ../src/api/callback.cc:125:void node::InternalCallbackScope::Close(): Assertion `(env_->execution_async_id()) == (0)' failed.
 1: 0x10dde07e5 node::Abort() (.cold.1) [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 2: 0x10ca93b39 node::Abort() [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 3: 0x10ca939a1 node::Assert(node::AssertionInfo const&) [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 4: 0x10c9c3cdd node::InternalCallbackScope::Close() [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 5: 0x10c9c35de node::InternalCallbackScope::~InternalCallbackScope() [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 6: 0x10ca5f4a3 node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 7: 0x10c9c81fd node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 8: 0x10cad53f0 node::NodeMainInstance::Run(node::EnvSerializeInfo const*) [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
 9: 0x10ca6197d node::Start(int, char**) [/Users/jon/.nvm/versions/node/v16.2.0/bin/node]
10: 0x7fff20653f3d start [/usr/lib/system/libdyld.dylib]
11: 0x2 
error Command failed with signal "SIGABRT".

Additional information

To be honest I don't understand the finer points of this crash, but I'm creating this issue because my perception is that upgrading from Node 16.1 to 16.2 introduces this regression. I'm happy to add more context or try different solutions if that will be helpful.

And I want to take a moment to thank the maintainers of Node for making these releases available, crashing or not I'm grateful for your hard work.

@gaggle gaggle changed the title Node 16.2 regression, crashing with node::InternalCallbackScope::Close(): Assertion (env_->execution_async_id()) == (0)' failed` Node 16.2 regression, crashing with node::InternalCallbackScope::Close(): Assertion (env_->execution_async_id()) == (0)' failed May 26, 2021
@gaggle gaggle changed the title Node 16.2 regression, crashing with node::InternalCallbackScope::Close(): Assertion (env_->execution_async_id()) == (0)' failed Node 16.2 regression, crashing with node::InternalCallbackScope::Close(): Assertion (env_->execution_async_id()) == (0) failed May 26, 2021
@gaggle gaggle changed the title Node 16.2 regression, crashing with node::InternalCallbackScope::Close(): Assertion (env_->execution_async_id()) == (0) failed Node 16.2 regression, crashing with node::InternalCallbackScope::Close() error May 26, 2021
@Ayase-252 Ayase-252 added the async_hooks Issues and PRs related to the async hooks subsystem. label May 26, 2021
@aduh95
Copy link
Contributor

aduh95 commented May 26, 2021

A repro that doesn't depend on any npm package would be very helpful.

@Ayase-252
Copy link
Member

Reproduciable in 16.2.0 but not in master. It may have been fixed. I'd retest it when 16.3.0 is released.

@hongbo-miao
Copy link

hongbo-miao commented May 27, 2021

Met same issue. Another demo to help reproduce at #38814

@gaggle
Copy link
Author

gaggle commented May 28, 2021

A repro that doesn't depend on any npm package would be very helpful.

Just to say I did try to remove the two dependencies, but I couldn't find a way to still trigger the error. I suspect there is a way if I better understood the underlying trigger, but given my current understanding I couldn't find it. Sorry, I do appreciate external dependencies are not the responsibility of this project, I hope someone can see the pattern well enough to narrow down the trigger.

Qard pushed a commit to Qard/node that referenced this issue Jun 3, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Qard pushed a commit to Qard/node that referenced this issue Jun 3, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Qard pushed a commit to Qard/node that referenced this issue Jun 3, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Qard pushed a commit to Qard/node that referenced this issue Jun 3, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Qard pushed a commit to Qard/node that referenced this issue Jun 3, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Qard pushed a commit to Qard/node that referenced this issue Jun 3, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
@Flarna Flarna closed this as completed in 1c36eef Jun 5, 2021
@Qard
Copy link
Member

Qard commented Jun 6, 2021

This should be fixed by #38912 which will be in the next release, likely 16.3.1 or 16.4.0. If you have any further issues, let me know!

targos pushed a commit that referenced this issue Jun 11, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes #38814
Fixes #38815
Refs #36394

PR-URL: #38912
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Qard pushed a commit to Qard/node that referenced this issue Jun 15, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Qard pushed a commit to Qard/node that referenced this issue Jul 19, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Qard pushed a commit to Qard/node that referenced this issue Jul 21, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Qard pushed a commit to Qard/node that referenced this issue Jul 21, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Qard pushed a commit to Qard/node that referenced this issue Jul 21, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Qard pushed a commit to Qard/node that referenced this issue Aug 1, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
targos pushed a commit that referenced this issue Aug 3, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes #38814
Fixes #38815
Refs #36394

PR-URL: #38912
Backport-PR-URL: #38577
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
BethGriggs pushed a commit that referenced this issue Aug 12, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes #38814
Fixes #38815
Refs #36394

PR-URL: #38912
Backport-PR-URL: #38577
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
MylesBorins pushed a commit that referenced this issue Aug 31, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes #38814
Fixes #38815
Refs #36394

PR-URL: #38912
Backport-PR-URL: #38577
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
foxxyz pushed a commit to foxxyz/node that referenced this issue Oct 18, 2021
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Backport-PR-URL: nodejs#38577
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants