src: manage handles more explicitly in cares_wrap, node_stat_watcher

src/cares_wrap.cc

@@ -151,7 +151,8 @@ class ChannelWrap : public AsyncWrap {
 
   void Setup();
   void EnsureServers();
-  void CleanupTimer();
+  void StartTimer();
+  void CloseTimer();
 
   void ModifyActivityQueryCount(int count);
 
@@ -313,13 +314,7 @@ void ares_sockstate_cb(void* data,
   if (read || write) {
     if (!task) {
       /* New socket */
-
-      /* If this is the first socket then start the timer. */
-      uv_timer_t* timer_handle = channel->timer_handle();
-      if (!uv_is_active(reinterpret_cast<uv_handle_t*>(timer_handle))) {
-        CHECK(channel->task_list()->empty());
-        uv_timer_start(timer_handle, ChannelWrap::AresTimeout, 1000, 1000);
-      }
+      channel->StartTimer();
 
       task = ares_task_create(channel, sock);
       if (task == nullptr) {
@@ -349,7 +344,7 @@ void ares_sockstate_cb(void* data,
     channel->env()->CloseHandle(&task->poll_watcher, ares_poll_close_cb);
 
     if (channel->task_list()->empty()) {
-      uv_timer_stop(channel->timer_handle());
+      channel->CloseTimer();
     }
   }
 }
@@ -490,15 +485,26 @@ void ChannelWrap::Setup() {
   }
 
   library_inited_ = true;
+}
 
-  /* Initialize the timeout timer. The timer won't be started until the */
-  /* first socket is opened. */
-  CleanupTimer();
-  timer_handle_ = new uv_timer_t();
-  timer_handle_->data = static_cast<void*>(this);
-  uv_timer_init(env()->event_loop(), timer_handle_);
+void ChannelWrap::StartTimer() {
+  if (timer_handle_ == nullptr) {
+    timer_handle_ = new uv_timer_t();
+    timer_handle_->data = static_cast<void*>(this);
+    uv_timer_init(env()->event_loop(), timer_handle_);
+  } else if (uv_is_active(reinterpret_cast<uv_handle_t*>(timer_handle_))) {
+    return;
+  }
+  uv_timer_start(timer_handle_, AresTimeout, 1000, 1000);
 }
 
+void ChannelWrap::CloseTimer() {
+  if (timer_handle_ == nullptr)
+    return;
+
+  env()->CloseHandle(timer_handle_, [](uv_timer_t* handle) { delete handle; });
+  timer_handle_ = nullptr;
+}
 
 ChannelWrap::~ChannelWrap() {
   if (library_inited_) {
@@ -508,17 +514,10 @@ ChannelWrap::~ChannelWrap() {
   }
 
   ares_destroy(channel_);
-  CleanupTimer();
+  CloseTimer();
 }
 
 
-void ChannelWrap::CleanupTimer() {
-  if (timer_handle_ == nullptr) return;
-
-  env()->CloseHandle(timer_handle_, [](uv_timer_t* handle) { delete handle; });
-  timer_handle_ = nullptr;
-}
-
 void ChannelWrap::ModifyActivityQueryCount(int count) {
   active_query_count_ += count;
   if (active_query_count_ < 0) active_query_count_ = 0;
@@ -566,6 +565,7 @@ void ChannelWrap::EnsureServers() {
   /* destroy channel and reset channel */
   ares_destroy(channel_);
 
+  CloseTimer();
   Setup();
 }
 

src/node_stat_watcher.cc

@@ -77,19 +77,15 @@ void StatWatcher::Initialize(Environment* env, Local<Object> target) {
 
 StatWatcher::StatWatcher(Environment* env, Local<Object> wrap, bool use_bigint)
     : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_STATWATCHER),
-      watcher_(new uv_fs_poll_t),
+      watcher_(nullptr),
       use_bigint_(use_bigint) {
   MakeWeak();
-  uv_fs_poll_init(env->event_loop(), watcher_);
-  watcher_->data = static_cast<void*>(this);
 }
 
 
 StatWatcher::~StatWatcher() {
-  if (IsActive()) {
+  if (IsActive())
     Stop();
-  }
-  env()->CloseHandle(watcher_, [](uv_fs_poll_t* handle) { delete handle; });
 }
 
 
@@ -123,7 +119,7 @@ void StatWatcher::New(const FunctionCallbackInfo<Value>& args) {
 }
 
 bool StatWatcher::IsActive() {
-  return uv_is_active(reinterpret_cast<uv_handle_t*>(watcher_)) != 0;
+  return watcher_ != nullptr;
 }
 
 void StatWatcher::IsActive(const v8::FunctionCallbackInfo<v8::Value>& args) {
@@ -156,6 +152,9 @@ void StatWatcher::Start(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[2]->IsUint32());
   const uint32_t interval = args[2].As<Uint32>()->Value();
 
+  wrap->watcher_ = new uv_fs_poll_t();
+  CHECK_EQ(0, uv_fs_poll_init(wrap->env()->event_loop(), wrap->watcher_));
+  wrap->watcher_->data = static_cast<void*>(wrap);
   // Safe, uv_ref/uv_unref are idempotent.
   if (persistent)
     uv_ref(reinterpret_cast<uv_handle_t*>(wrap->watcher_));
@@ -187,7 +186,8 @@ void StatWatcher::Stop(const FunctionCallbackInfo<Value>& args) {
 
 
 void StatWatcher::Stop() {
-  uv_fs_poll_stop(watcher_);
+  env()->CloseHandle(watcher_, [](uv_fs_poll_t* handle) { delete handle; });
+  watcher_ = nullptr;
   MakeWeak();
 }
 

test/sequential/test-async-wrap-getasyncid.js

@@ -1,4 +1,5 @@
 'use strict';
+// Flags: --expose-gc
 
 const common = require('../common');
 const assert = require('assert');
@@ -22,6 +23,11 @@ common.crashOnUnhandledRejection();
     },
   }).enable();
   process.on('beforeExit', common.mustCall(() => {
+    // This garbage collection call verifies that the wraps being garbage
+    // collected doesn't resurrect the process again due to weirdly timed
+    // uv_close calls and other similar instruments in destructors.
+    global.gc();
+
     process.removeAllListeners('uncaughtException');
     hooks.disable();
     delete providers.NONE;  // Should never be used.