src: fix node watchdog race condition

PR-URL: #43780Fixes: #43699
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Author: theanarkh

src/node_watchdog.cc

@@ -102,6 +102,7 @@ void Watchdog::Timer(uv_timer_t* timer) {
 SigintWatchdog::SigintWatchdog(
  v8::Isolate* isolate, bool* received_signal)
  : isolate_(isolate), received_signal_(received_signal) {
+ Mutex::ScopedLock lock(SigintWatchdogHelper::GetInstanceActionMutex());
 // Register this watchdog with the global SIGINT/Ctrl+C listener.
 SigintWatchdogHelper::GetInstance()->Register(this);
 // Start the helper thread, if that has not already happened.
@@ -110,6 +111,7 @@ SigintWatchdog::SigintWatchdog(
 
 
 SigintWatchdog::~SigintWatchdog() {
+ Mutex::ScopedLock lock(SigintWatchdogHelper::GetInstanceActionMutex());
 SigintWatchdogHelper::GetInstance()->Unregister(this);
 SigintWatchdogHelper::GetInstance()->Stop();
 }
@@ -144,6 +146,7 @@ void TraceSigintWatchdog::New(const FunctionCallbackInfo<Value>& args) {
 voidTraceSigintWatchdog::Start(const FunctionCallbackInfo<Value>& args) {
  TraceSigintWatchdog* watchdog;
 ASSIGN_OR_RETURN_UNWRAP(&watchdog, args.Holder());
+ Mutex::ScopedLock lock(SigintWatchdogHelper::GetInstanceActionMutex());
 // Register this watchdog with the global SIGINT/Ctrl+C listener.
 SigintWatchdogHelper::GetInstance()->Register(watchdog);
 // Start the helper thread, if that has not already happened.
@@ -154,6 +157,7 @@ void TraceSigintWatchdog::Start(const FunctionCallbackInfo<Value>& args) {
 voidTraceSigintWatchdog::Stop(const FunctionCallbackInfo<Value>& args) {
  TraceSigintWatchdog* watchdog;
 ASSIGN_OR_RETURN_UNWRAP(&watchdog, args.Holder());
+ Mutex::ScopedLock lock(SigintWatchdogHelper::GetInstanceActionMutex());
 SigintWatchdogHelper::GetInstance()->Unregister(watchdog);
 SigintWatchdogHelper::GetInstance()->Stop();
 }
@@ -215,6 +219,7 @@ void TraceSigintWatchdog::HandleInterrupt() {
  signal_flag_ = SignalFlags::None;
  interrupting = false;
 
+ Mutex::ScopedLock lock(SigintWatchdogHelper::GetInstanceActionMutex());
 SigintWatchdogHelper::GetInstance()->Unregister(this);
 SigintWatchdogHelper::GetInstance()->Stop();
 raise(SIGINT);
@@ -413,6 +418,7 @@ SigintWatchdogHelper::~SigintWatchdogHelper() {
 }
 
 SigintWatchdogHelper SigintWatchdogHelper::instance;
+Mutex SigintWatchdogHelper::instance_action_mutex_;
 
 namespacewatchdog {
 staticvoidInitialize(Local<Object> target,

src/node_watchdog.h

@@ -110,6 +110,7 @@ class TraceSigintWatchdog : public HandleWrap, public SigintWatchdogBase {
 classSigintWatchdogHelper {
 public:
 static SigintWatchdogHelper* GetInstance() { return &instance; }
+static Mutex& GetInstanceActionMutex() { return instance_action_mutex_; }
 voidRegister(SigintWatchdogBase* watchdog);
 voidUnregister(SigintWatchdogBase* watchdog);
 boolHasPendingSignal();
@@ -123,6 +124,7 @@ class SigintWatchdogHelper {
 
 staticboolInformWatchdogsAboutSignal();
 static SigintWatchdogHelper instance;
+static Mutex instance_action_mutex_;
 
 int start_stop_count_;
 

test/parallel/test-vm-break-on-sigint.js

@@ -0,0 +1,22 @@
+'use strict';
+constcommon=require('../common');
+
+// This test ensures that running vm with breakOnSignt option in multiple
+// worker_threads does not crash.
+// Issue: https://github.com/nodejs/node/issues/43699
+const{ Worker }=require('worker_threads');
+constvm=require('vm');
+
+// Don't use isMainThread to allow running this test inside a worker.
+if(!process.env.HAS_STARTED_WORKER){
+process.env.HAS_STARTED_WORKER=1;
+for(leti=0;i<10;i++){
+constworker=newWorker(__filename);
+worker.on('exit',common.mustCall());
+}
+}else{
+constctx=vm.createContext({});
+for(leti=0;i<10000;i++){
+vm.runInContext('console.log(1)',ctx,{breakOnSigint: true});
+}
+}