v13.2.0
Linux nicolo-XPS-15-9570 5.3.0-23-generic #25-Ubuntu SMP Tue Nov 12 09:22:33 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
After upgrading from node 13.1.0 to 13.2.0, after running lerna
node doesn't exit normally but crashes:
lerna success Bootstrapped 155 packages
/home/nicolo/n/bin/node[20139]: ../src/signal_wrap.cc:159:void node::DecreaseSignalHandlerCount(int): Assertion `(new_handler_count) >= (0)' failed.
1: 0x9f0390 node::Abort() [/home/nicolo/n/bin/node]
2: 0x9f0417 [/home/nicolo/n/bin/node]
3: 0xa91bdc node::DecreaseSignalHandlerCount(int) [/home/nicolo/n/bin/node]
4: 0xa91cb4 [/home/nicolo/n/bin/node]
5: 0x98fbd5 node::Environment::CleanupHandles() [/home/nicolo/n/bin/node]
6: 0x98fe6b node::Environment::RunCleanup() [/home/nicolo/n/bin/node]
7: 0xa2d2f0 node::NodeMainInstance::Run() [/home/nicolo/n/bin/node]
8: 0x9c1311 node::Start(int, char**) [/home/nicolo/n/bin/node]
9: 0x7f14c1fd71e3 __libc_start_main [/lib/x86_64-linux-gnu/libc.so.6]
10: 0x95ed25 [/home/nicolo/n/bin/node]
Aborted (core dumped)
Steps to reproduce this bug: (you need make
and yarn
)
git clone https://github.com/babel/babel.git
cd babel
make bootstrap
cc @addaleax (author of 55f98df303939774639bb597c6392c1c85bae6dd, which introduced the assertion)
Here’s a quick fix if anybody wants to open a PR and get 13.2.1 out:
```diff
diff --git a/src/signal_wrap.cc b/src/signal_wrap.cc
index cf67dc590f6d..bc2d9f1e355e 100644
--- a/src/signal_wrap.cc
+++ b/src/signal_wrap.cc
@@ -91,7 +91,10 @@ class SignalWrap : public HandleWrap {
}
void Close(v8::Local
- if (active_) DecreaseSignalHandlerCount(handle_.signum);
+ if (active_) {
+ DecreaseSignalHandlerCount(handle_.signum);
+ active_ = false;
+ }
HandleWrap::Close(close_callback);
}
```
I’ll try to look more into why this is happening tomorrow, it’s an odd bug and points to us attempting to close a signal handle multiple times.
seems like #30582 fixes it
Thanks! ❤️
When can we see this in a patch release?
That depends on @nodejs/releasers – I figure we’d want to get #30586 resolved soon too, so maybe after that?
Fwiw, it should work well enough as a temporary workaround to use something like
process.prependListener('exit', () => process.removeListener = () => {});
@nodejs/releasers Can somebody do a v13.x this week?
@nodejs/releasers Can somebody do a v13.x this week?
@BridgeAR signed up for one this week: https://github.com/nodejs/Release/issues/487
Most helpful comment
Here’s a quick fix if anybody wants to open a PR and get 13.2.1 out:
```diff
diff --git a/src/signal_wrap.cc b/src/signal_wrap.cc
index cf67dc590f6d..bc2d9f1e355e 100644
--- a/src/signal_wrap.cc
+++ b/src/signal_wrap.cc
@@ -91,7 +91,10 @@ class SignalWrap : public HandleWrap {
}
void Close(v8::Local close_callback) override {
- if (active_) DecreaseSignalHandlerCount(handle_.signum);
+ if (active_) {
+ DecreaseSignalHandlerCount(handle_.signum);
+ active_ = false;
+ }
HandleWrap::Close(close_callback);
}
```
I’ll try to look more into why this is happening tomorrow, it’s an odd bug and points to us attempting to close a signal handle multiple times.