vm: fix race condition in timeout

orangemocha · tjfontaine · commit cdc038ceb60c · 2013-12-18T15:16:36.000-08:00
Eliminate a race condition between uv_async_send and the closing of the corresponding handle. Also made errors in Watchdog constructor call abort() Fixes nodejs#6088
diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc
@@ -20,34 +20,29 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "node_watchdog.h"
+#include "util.h"
 #include <assert.h>
 
 namespace node {
 
 using v8::V8;
 
 
-Watchdog::Watchdog(uint64_t ms) : thread_created_(false), destroyed_(false) {
+Watchdog::Watchdog(uint64_t ms) : destroyed_(false) {
   loop_ = uv_loop_new();
-  if (!loop_)
-    return;
+  CHECK(loop_);
 
   int rc = uv_async_init(loop_, &async_, &Watchdog::Async);
-  assert(rc == 0);
+  CHECK(0 == rc);  // NOLINT(readability/check)
 
   rc = uv_timer_init(loop_, &timer_);
-  assert(rc == 0);
+  CHECK(0 == rc);  // NOLINT(readability/check)
 
   rc = uv_timer_start(&timer_, &Watchdog::Timer, ms, 0);
-  if (rc) {
-    return;
-  }
+  CHECK(0 == rc);  // NOLINT(readability/check)
 
   rc = uv_thread_create(&thread_, &Watchdog::Run, this);
-  if (rc) {
-    return;
-  }
-  thread_created_ = true;
+  CHECK(0 == rc);  // NOLINT(readability/check)
 }
 
 
@@ -66,14 +61,15 @@ void Watchdog::Destroy() {
     return;
   }
 
-  if (thread_created_) {
-    uv_async_send(&async_);
-    uv_thread_join(&thread_);
-  }
+  uv_async_send(&async_);
+  uv_thread_join(&thread_);
 
-  if (loop_) {
-    uv_loop_delete(loop_);
-  }
+  uv_close(reinterpret_cast<uv_handle_t*>(&async_), NULL);
+
+  // UV_RUN_DEFAULT so that libuv has a chance to clean up.
+  uv_run(loop_, UV_RUN_DEFAULT);
+
+  uv_loop_delete(loop_);
 
   destroyed_ = true;
 }
@@ -86,11 +82,8 @@ void Watchdog::Run(void* arg) {
   uv_run(wd->loop_, UV_RUN_ONCE);
 
   // Loop ref count reaches zero when both handles are closed.
-  uv_close(reinterpret_cast<uv_handle_t*>(&wd->async_), NULL);
+  // Close the timer handle on this side and let Destroy() close async_
   uv_close(reinterpret_cast<uv_handle_t*>(&wd->timer_), NULL);
-
-  // UV_RUN_DEFAULT so that libuv has a chance to clean up.
-  uv_run(wd->loop_, UV_RUN_DEFAULT);
 }
 
 
diff --git a/src/node_watchdog.h b/src/node_watchdog.h
@@ -45,7 +45,6 @@ class Watchdog {
   uv_loop_t* loop_;
   uv_async_t async_;
   uv_timer_t timer_;
-  bool thread_created_;
   bool destroyed_;
 };
 
