[13.2.0] Assertion `(new_handler_count) >= (0)' failed

[nicolo-ribaudo]

• Version: v13.2.0
• Platform: 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
• Subsystem: ?

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 55f98df, which introduced the assertion)

[addaleax]

Here’s a quick fix if anybody wants to open a PR and get 13.2.1 out:

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<v8::Value> 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.

[ljharb]

seems like #30582 fixes it

[codebytere]

Closed by #30582.

[nicolo-ribaudo]

Thanks! ❤️

[ljharb]

When can we see this in a patch release?

[addaleax]

That depends on @nodejs/releasers – I figure we’d want to get #30586 resolved soon too, so maybe after that?

[addaleax]

Fwiw, it should work well enough as a temporary workaround to use something like

process.prependListener('exit', () => process.removeListener = () => {});

[addaleax]

@nodejs/releasers Can somebody do a v13.x this week?

[richardlau]

@nodejs/releasers Can somebody do a v13.x this week?

@BridgeAR signed up for one this week: nodejs/Release#487