src: simplify handlewrap state tracking logic

This also updates the tests to expect that a closed handle has no reference count. PR-URL: #6395 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Apr 27, 2016 · a58d483 · a58d483
1 parent cad1a62
commit a58d483
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 17 deletions.
diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc
@@ -35,10 +35,7 @@ void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args) {
 
 void HandleWrap::Unrefed(const FunctionCallbackInfo<Value>& args) {
   HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
-  // XXX(bnoordhuis) It's debatable whether a nullptr wrap should count
-  // as having a reference count but it's compatible with the logic that
-  // it replaces.
-  args.GetReturnValue().Set(wrap == nullptr || !HasRef(wrap));
+  args.GetReturnValue().Set(!HasRef(wrap));
 }
 
 
@@ -51,6 +48,9 @@ void HandleWrap::Close(const FunctionCallbackInfo<Value>& args) {
   if (!IsAlive(wrap))
     return;
 
+  if (wrap->state_ != kInitialized)
+    return;
+
   CHECK_EQ(false, wrap->persistent().IsEmpty());
   uv_close(wrap->handle__, OnClose);
   wrap->state_ = kClosing;

diff --git a/src/handle_wrap.h b/src/handle_wrap.h
@@ -38,15 +38,11 @@ class HandleWrap : public AsyncWrap {
   static void Unrefed(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   static inline bool IsAlive(const HandleWrap* wrap) {
-    // XXX(bnoordhuis) It's debatable whether only kInitialized should
-    // count as alive but it's compatible with the check that it replaces.
-    return wrap != nullptr && wrap->state_ == kInitialized;
+    return wrap != nullptr && wrap->state_ != kClosed;
   }
 
   static inline bool HasRef(const HandleWrap* wrap) {
-    return wrap != nullptr &&
-           wrap->state_ != kClosed &&
-           uv_has_ref(wrap->GetHandle());
+    return IsAlive(wrap) && uv_has_ref(wrap->GetHandle());
   }
 
   inline uv_handle_t* GetHandle() const { return handle__; }

diff --git a/test/parallel/test-handle-wrap-isrefed-tty.js b/test/parallel/test-handle-wrap-isrefed-tty.js
@@ -19,7 +19,7 @@ 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)));
   assert(tty._handle.unrefed(), true);
   return;
 }

diff --git a/test/parallel/test-handle-wrap-isrefed.js b/test/parallel/test-handle-wrap-isrefed.js
@@ -22,7 +22,7 @@ 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)));
   assert(cp._handle.unrefed(), false);
 }
 
@@ -39,7 +39,8 @@ 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)));
   assert(sock4._handle.unrefed(), false);
 
   const sock6 = dgram.createSocket('udp6');
@@ -49,7 +50,8 @@ 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)));
   assert(sock6._handle.unrefed(), false);
 }
 
@@ -65,7 +67,7 @@ function makeAssert(message) {
   assert(handle.unrefed(), true);
   handle.ref();
   assert(handle.unrefed(), false);
-  handle.close();
+  handle.close(common.mustCall(() => assert(handle.unrefed(), true)));
   assert(handle.unrefed(), false);
 }
 
@@ -84,7 +86,8 @@ 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)));
   assert(server._handle.unrefed(), false);
 }
 
@@ -98,6 +101,7 @@ 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)));
   assert(timer._handle.unrefed(), false);
 }