From 249365524ebd6e50f6aa375ea05b33731445e94d Mon Sep 17 00:00:00 2001 From: theanarkh Date: Fri, 15 Jul 2022 07:56:39 +0800 Subject: [PATCH] src: fix node watchdog race condition PR-URL: https://github.com/nodejs/node/pull/43780 Fixes: https://github.com/nodejs/node/issues/43699 Reviewed-By: Darshan Sen Reviewed-By: Antoine du Hamel --- src/node_watchdog.cc | 6 ++++++ src/node_watchdog.h | 2 ++ test/parallel/test-vm-break-on-sigint.js | 22 ++++++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100644 test/parallel/test-vm-break-on-sigint.js diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc index ff2a0229087138..31c8f744a3b320 100644 --- a/src/node_watchdog.cc +++ b/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& args) { void TraceSigintWatchdog::Start(const FunctionCallbackInfo& 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& args) { void TraceSigintWatchdog::Stop(const FunctionCallbackInfo& 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_; namespace watchdog { static void Initialize(Local target, diff --git a/src/node_watchdog.h b/src/node_watchdog.h index 8bccfceef3c521..ae70157224b1c7 100644 --- a/src/node_watchdog.h +++ b/src/node_watchdog.h @@ -110,6 +110,7 @@ class TraceSigintWatchdog : public HandleWrap, public SigintWatchdogBase { class SigintWatchdogHelper { public: static SigintWatchdogHelper* GetInstance() { return &instance; } + static Mutex& GetInstanceActionMutex() { return instance_action_mutex_; } void Register(SigintWatchdogBase* watchdog); void Unregister(SigintWatchdogBase* watchdog); bool HasPendingSignal(); @@ -123,6 +124,7 @@ class SigintWatchdogHelper { static bool InformWatchdogsAboutSignal(); static SigintWatchdogHelper instance; + static Mutex instance_action_mutex_; int start_stop_count_; diff --git a/test/parallel/test-vm-break-on-sigint.js b/test/parallel/test-vm-break-on-sigint.js new file mode 100644 index 00000000000000..6ad25bfed234b8 --- /dev/null +++ b/test/parallel/test-vm-break-on-sigint.js @@ -0,0 +1,22 @@ +'use strict'; +const common = 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'); +const vm = 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 (let i = 0; i < 10; i++) { + const worker = new Worker(__filename); + worker.on('exit', common.mustCall()); + } +} else { + const ctx = vm.createContext({}); + for (let i = 0; i < 10000; i++) { + vm.runInContext('console.log(1)', ctx, { breakOnSigint: true }); + } +}