src: use libuv's refcounting directly

src/handle_wrap.cc

@@ -20,28 +20,22 @@ using v8::Value;
 void HandleWrap::Ref(const FunctionCallbackInfo<Value>& args) {
   HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
 
-  if (IsAlive(wrap)) {
-    uv_ref(wrap->handle__);
-    wrap->flags_ &= ~kUnref;
-  }
+  if (IsAlive(wrap))
+    uv_ref(wrap->GetHandle());
 }
 
 
 void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args) {
   HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
 
-  if (IsAlive(wrap)) {
-    uv_unref(wrap->handle__);
-    wrap->flags_ |= kUnref;
-  }
+  if (IsAlive(wrap))
+    uv_unref(wrap->GetHandle());
 }
 
 
 void HandleWrap::Unrefed(const FunctionCallbackInfo<Value>& args) {
   HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
-
-  bool unrefed = wrap->flags_ & kUnref == 1;
-  args.GetReturnValue().Set(unrefed);
+  args.GetReturnValue().Set(!HasRef(wrap));
 }
 
 
@@ -50,17 +44,20 @@ void HandleWrap::Close(const FunctionCallbackInfo<Value>& args) {
 
   HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
 
-  // guard against uninitialized handle or double close
+  // Guard against uninitialized handle or double close.
   if (!IsAlive(wrap))
     return;
 
+  if (wrap->state_ != kInitialized)
+    return;
+
   CHECK_EQ(false, wrap->persistent().IsEmpty());
   uv_close(wrap->handle__, OnClose);
-  wrap->handle__ = nullptr;
+  wrap->state_ = kClosing;
 
   if (args[0]->IsFunction()) {
     wrap->object()->Set(env->onclose_string(), args[0]);
-    wrap->flags_ |= kCloseCallback;
+    wrap->state_ = kClosingWithCallback;
   }
 }
 
@@ -71,7 +68,7 @@ HandleWrap::HandleWrap(Environment* env,
                        AsyncWrap::ProviderType provider,
                        AsyncWrap* parent)
     : AsyncWrap(env, object, provider, parent),
-      flags_(0),
+      state_(kInitialized),
       handle__(handle) {
   handle__->data = this;
   HandleScope scope(env->isolate());
@@ -89,22 +86,19 @@ void HandleWrap::OnClose(uv_handle_t* handle) {
   HandleWrap* wrap = static_cast<HandleWrap*>(handle->data);
   Environment* env = wrap->env();
   HandleScope scope(env->isolate());
+  Context::Scope context_scope(env->context());
 
   // The wrap object should still be there.
   CHECK_EQ(wrap->persistent().IsEmpty(), false);
+  CHECK(wrap->state_ >= kClosing && wrap->state_ <= kClosingWithCallback);
 
-  // But the handle pointer should be gone.
-  CHECK_EQ(wrap->handle__, nullptr);
-
-  HandleScope handle_scope(env->isolate());
-  Context::Scope context_scope(env->context());
-  Local<Object> object = wrap->object();
+  const bool have_close_callback = (wrap->state_ == kClosingWithCallback);
+  wrap->state_ = kClosed;
 
-  if (wrap->flags_ & kCloseCallback) {
+  if (have_close_callback)
     wrap->MakeCallback(env->onclose_string(), 0, nullptr);
-  }
 
-  object->SetAlignedPointerInInternalField(0, nullptr);
+  wrap->object()->SetAlignedPointerInInternalField(0, nullptr);
   wrap->persistent().Reset();
   delete wrap;
 }

src/handle_wrap.h

@@ -38,7 +38,11 @@ class HandleWrap : public AsyncWrap {
   static void Unrefed(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   static inline bool IsAlive(const HandleWrap* wrap) {
-    return wrap != nullptr && wrap->GetHandle() != nullptr;
+    return wrap != nullptr && wrap->state_ != kClosed;
+  }
+
+  static inline bool HasRef(const HandleWrap* wrap) {
+    return IsAlive(wrap) && uv_has_ref(wrap->GetHandle());
   }
 
   inline uv_handle_t* GetHandle() const { return handle__; }
@@ -56,13 +60,10 @@ class HandleWrap : public AsyncWrap {
   friend void GetActiveHandles(const v8::FunctionCallbackInfo<v8::Value>&);
   static void OnClose(uv_handle_t* handle);
   ListNode<HandleWrap> handle_wrap_queue_;
-  unsigned int flags_;
+  enum { kInitialized, kClosing, kClosingWithCallback, kClosed } state_;
   // Using double underscore due to handle_ member in tcp_wrap. Probably
   // tcp_wrap should rename it's member to 'handle'.
-  uv_handle_t* handle__;
-
-  static const unsigned int kUnref = 1;
-  static const unsigned int kCloseCallback = 2;
+  uv_handle_t* const handle__;
 };
 
 

src/node.cc

@@ -1723,7 +1723,7 @@ void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
   Local<String> owner_sym = env->owner_string();
 
   for (auto w : *env->handle_wrap_queue()) {
-    if (w->persistent().IsEmpty() || (w->flags_ & HandleWrap::kUnref))
+    if (w->persistent().IsEmpty() || !HandleWrap::HasRef(w))
       continue;
     Local<Object> object = w->object();
     Local<Value> owner = object->Get(owner_sym);

test/parallel/test-handle-wrap-isrefed-tty.js

@@ -19,7 +19,8 @@ if (process.argv[2] === 'child') {
   assert(tty._handle.unrefed(), false);
   tty.unref();
   assert(tty._handle.unrefed(), true);
-  tty._handle.close();
+  tty._handle.close(common.mustCall(() => assert(tty._handle.unrefed(), true)));
+  tty._handle.close(common.fail);
   assert(tty._handle.unrefed(), true);
   return;
 }

test/parallel/test-handle-wrap-isrefed.js

@@ -22,7 +22,8 @@ function makeAssert(message) {
   assert(cp._handle.unrefed(), true);
   cp.ref();
   assert(cp._handle.unrefed(), false);
-  cp._handle.close();
+  cp._handle.close(common.mustCall(() => assert(cp._handle.unrefed(), true)));
+  cp._handle.close(common.fail);
   assert(cp._handle.unrefed(), false);
 }
 
@@ -39,7 +40,9 @@ function makeAssert(message) {
   assert(sock4._handle.unrefed(), true);
   sock4.ref();
   assert(sock4._handle.unrefed(), false);
-  sock4._handle.close();
+  sock4._handle.close(
+      common.mustCall(() => assert(sock4._handle.unrefed(), true)));
+  sock4._handle.close(common.fail);
   assert(sock4._handle.unrefed(), false);
 
   const sock6 = dgram.createSocket('udp6');
@@ -49,7 +52,9 @@ function makeAssert(message) {
   assert(sock6._handle.unrefed(), true);
   sock6.ref();
   assert(sock6._handle.unrefed(), false);
-  sock6._handle.close();
+  sock6._handle.close(
+      common.mustCall(() => assert(sock6._handle.unrefed(), true)));
+  sock6._handle.close(common.fail);
   assert(sock6._handle.unrefed(), false);
 }
 
@@ -65,7 +70,8 @@ function makeAssert(message) {
   assert(handle.unrefed(), true);
   handle.ref();
   assert(handle.unrefed(), false);
-  handle.close();
+  handle.close(common.mustCall(() => assert(handle.unrefed(), true)));
+  handle.close(common.fail);
   assert(handle.unrefed(), false);
 }
 
@@ -84,7 +90,9 @@ function makeAssert(message) {
   server.ref();
   assert(server._handle.unrefed(), false);
   assert(server._unref, false);
-  server._handle.close();
+  server._handle.close(
+      common.mustCall(() => assert(server._handle.unrefed(), true)));
+  server._handle.close(common.fail);
   assert(server._handle.unrefed(), false);
 }
 
@@ -98,6 +106,8 @@ function makeAssert(message) {
   assert(timer._handle.unrefed(), true);
   timer.ref();
   assert(timer._handle.unrefed(), false);
-  timer.close();
+  timer._handle.close(
+      common.mustCall(() => assert(timer._handle.unrefed(), true)));
+  timer._handle.close(common.fail);
   assert(timer._handle.unrefed(), false);
 }