test,vm: fix flaky test-util-sigint-watchdog

src/node_util.cc

@@ -113,6 +113,13 @@ void StopSigintWatchdog(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(had_pending_signals);
 }
 
+
+void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) {
+  bool ret = SigintWatchdogHelper::GetInstance()->HasPendingSignal();
+  args.GetReturnValue().Set(ret);
+}
+
+
 void Initialize(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> context) {
@@ -138,6 +145,7 @@ void Initialize(Local<Object> target,
 
   env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog);
   env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog);
+  env->SetMethod(target, "watchdogHasPendingSigint", WatchdogHasPendingSigint);
 }
 
 }  // namespace util

src/node_watchdog.cc

@@ -160,7 +160,7 @@ BOOL WINAPI SigintWatchdogHelper::WinCtrlCHandlerRoutine(DWORD dwCtrlType) {
 
 
 bool SigintWatchdogHelper::InformWatchdogsAboutSignal() {
-  uv_mutex_lock(&instance.list_mutex_);
+  Mutex::ScopedLock list_lock(instance.list_mutex_);
 
   bool is_stopping = false;
 #ifdef __POSIX__
@@ -176,17 +176,15 @@ bool SigintWatchdogHelper::InformWatchdogsAboutSignal() {
   for (auto it : instance.watchdogs_)
     it->HandleSigint();
 
-  uv_mutex_unlock(&instance.list_mutex_);
   return is_stopping;
 }
 
 
 int SigintWatchdogHelper::Start() {
-  int ret = 0;
-  uv_mutex_lock(&mutex_);
+  Mutex::ScopedLock lock(mutex_);
 
   if (start_stop_count_++ > 0) {
-    goto dont_start;
+    return 0;
   }
 
 #ifdef __POSIX__
@@ -197,10 +195,10 @@ int SigintWatchdogHelper::Start() {
   sigset_t sigmask;
   sigfillset(&sigmask);
   CHECK_EQ(0, pthread_sigmask(SIG_SETMASK, &sigmask, &sigmask));
-  ret = pthread_create(&thread_, nullptr, RunSigintWatchdog, nullptr);
+  int ret = pthread_create(&thread_, nullptr, RunSigintWatchdog, nullptr);
   CHECK_EQ(0, pthread_sigmask(SIG_SETMASK, &sigmask, nullptr));
   if (ret != 0) {
-    goto dont_start;
+    return ret;
   }
   has_running_thread_ = true;
 
@@ -209,34 +207,36 @@ int SigintWatchdogHelper::Start() {
   SetConsoleCtrlHandler(WinCtrlCHandlerRoutine, TRUE);
 #endif
 
- dont_start:
-  uv_mutex_unlock(&mutex_);
-  return ret;
+  return 0;
 }
 
 
 bool SigintWatchdogHelper::Stop() {
-  uv_mutex_lock(&mutex_);
-  uv_mutex_lock(&list_mutex_);
+  bool had_pending_signal;
+  Mutex::ScopedLock lock(mutex_);
 
-  bool had_pending_signal = has_pending_signal_;
+  {
+    Mutex::ScopedLock list_lock(list_mutex_);
 
-  if (--start_stop_count_ > 0) {
-    uv_mutex_unlock(&list_mutex_);
-    goto dont_stop;
-  }
+    had_pending_signal = has_pending_signal_;
+
+    if (--start_stop_count_ > 0) {
+      has_pending_signal_ = false;
+      return had_pending_signal;
+    }
 
 #ifdef __POSIX__
-  // Set stopping now because it's only protected by list_mutex_.
-  stopping_ = true;
+    // Set stopping now because it's only protected by list_mutex_.
+    stopping_ = true;
 #endif
 
-  watchdogs_.clear();
-  uv_mutex_unlock(&list_mutex_);
+    watchdogs_.clear();
+  }
 
 #ifdef __POSIX__
   if (!has_running_thread_) {
-    goto dont_stop;
+    has_pending_signal_ = false;
+    return had_pending_signal;
   }
 
   // Wake up the helper thread.
@@ -252,32 +252,33 @@ bool SigintWatchdogHelper::Stop() {
 #endif
 
   had_pending_signal = has_pending_signal_;
- dont_stop:
-  uv_mutex_unlock(&mutex_);
-
   has_pending_signal_ = false;
+
   return had_pending_signal;
 }
 
 
+bool SigintWatchdogHelper::HasPendingSignal() {
+  Mutex::ScopedLock lock(list_mutex_);
+
+  return has_pending_signal_;
+}
+
+
 void SigintWatchdogHelper::Register(SigintWatchdog* wd) {
-  uv_mutex_lock(&list_mutex_);
+  Mutex::ScopedLock lock(list_mutex_);
 
   watchdogs_.push_back(wd);
-
-  uv_mutex_unlock(&list_mutex_);
 }
 
 
 void SigintWatchdogHelper::Unregister(SigintWatchdog* wd) {
-  uv_mutex_lock(&list_mutex_);
+  Mutex::ScopedLock lock(list_mutex_);
 
   auto it = std::find(watchdogs_.begin(), watchdogs_.end(), wd);
 
   CHECK_NE(it, watchdogs_.end());
   watchdogs_.erase(it);
-
-  uv_mutex_unlock(&list_mutex_);
 }
 
 
@@ -289,9 +290,6 @@ SigintWatchdogHelper::SigintWatchdogHelper()
   stopping_ = false;
   CHECK_EQ(0, uv_sem_init(&sem_, 0));
 #endif
-
-  CHECK_EQ(0, uv_mutex_init(&mutex_));
-  CHECK_EQ(0, uv_mutex_init(&list_mutex_));
 }
 
 
@@ -303,9 +301,6 @@ SigintWatchdogHelper::~SigintWatchdogHelper() {
   CHECK_EQ(has_running_thread_, false);
   uv_sem_destroy(&sem_);
 #endif
-
-  uv_mutex_destroy(&mutex_);
-  uv_mutex_destroy(&list_mutex_);
 }
 
 SigintWatchdogHelper SigintWatchdogHelper::instance;

src/node_watchdog.h

@@ -5,6 +5,7 @@
 
 #include "v8.h"
 #include "uv.h"
+#include "node_mutex.h"
 #include <vector>
 
 #ifdef __POSIX__
@@ -62,6 +63,7 @@ class SigintWatchdogHelper {
   static SigintWatchdogHelper* GetInstance() { return &instance; }
   void Register(SigintWatchdog* watchdog);
   void Unregister(SigintWatchdog* watchdog);
+  bool HasPendingSignal();
 
   int Start();
   bool Stop();
@@ -75,8 +77,8 @@ class SigintWatchdogHelper {
 
   int start_stop_count_;
 
-  uv_mutex_t mutex_;
-  uv_mutex_t list_mutex_;
+  Mutex mutex_;
+  Mutex list_mutex_;
   std::vector<SigintWatchdog*> watchdogs_;
   bool has_pending_signal_;
 

test/parallel/test-util-sigint-watchdog.js

@@ -20,34 +20,41 @@ if (common.isWindows) {
   // Test with one call to the watchdog, one signal.
   binding.startSigintWatchdog();
   process.kill(process.pid, 'SIGINT');
-  setTimeout(common.mustCall(() => {
+  waitForPendingSignal(common.mustCall(() => {
     const hadPendingSignals = binding.stopSigintWatchdog();
     assert.strictEqual(hadPendingSignals, true);
     next();
-  }), common.platformTimeout(100));
+  }));
 },
 (next) => {
   // Nested calls are okay.
   binding.startSigintWatchdog();
   binding.startSigintWatchdog();
   process.kill(process.pid, 'SIGINT');
-  setTimeout(common.mustCall(() => {
+  waitForPendingSignal(common.mustCall(() => {
     const hadPendingSignals1 = binding.stopSigintWatchdog();
     const hadPendingSignals2 = binding.stopSigintWatchdog();
     assert.strictEqual(hadPendingSignals1, true);
     assert.strictEqual(hadPendingSignals2, false);
     next();
-  }), common.platformTimeout(100));
+  }));
 },
 () => {
   // Signal comes in after first call to stop.
   binding.startSigintWatchdog();
   binding.startSigintWatchdog();
   const hadPendingSignals1 = binding.stopSigintWatchdog();
   process.kill(process.pid, 'SIGINT');
-  setTimeout(common.mustCall(() => {
+  waitForPendingSignal(common.mustCall(() => {
     const hadPendingSignals2 = binding.stopSigintWatchdog();
     assert.strictEqual(hadPendingSignals1, false);
     assert.strictEqual(hadPendingSignals2, true);
-  }), common.platformTimeout(100));
+  }));
 }].reduceRight((a, b) => common.mustCall(b).bind(null, a))();
+
+function waitForPendingSignal(cb) {
+  if (binding.watchdogHasPendingSigint())
+    cb();
+  else
+    setTimeout(waitForPendingSignal, 10, cb);
+}