From 965274d384f4f33d93f6de653f33ee5b0598a27d Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Tue, 26 Apr 2016 12:01:46 +0200
Subject: [PATCH] src: use libuv's refcounting directly

Don't implement an additional reference counting scheme on top of libuv.
Libuv is the canonical source for that information so use it directly.

PR-URL: https://github.com/nodejs/node/pull/6395
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/handle_wrap.cc | 42 ++++++++++++++++++------------------------
 src/handle_wrap.h  | 17 +++++++++++------
 src/node.cc        |  2 +-
 3 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc
index 7c47f85fd989a4..1a403974300d41 100644
--- a/src/handle_wrap.cc
+++ b/src/handle_wrap.cc
@@ -21,28 +21,25 @@ 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);
+  // 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));
 }
 
 
@@ -51,17 +48,17 @@ 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;
 
   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;
   }
 }
 
@@ -72,7 +69,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());
@@ -90,22 +87,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);
+  const bool have_close_callback = (wrap->state_ == kClosingWithCallback);
+  wrap->state_ = kClosed;
 
-  HandleScope handle_scope(env->isolate());
-  Context::Scope context_scope(env->context());
-  Local<Object> object = wrap->object();
-
-  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;
 }
diff --git a/src/handle_wrap.h b/src/handle_wrap.h
index d945143d31a952..fe3c5a8d9dc243 100644
--- a/src/handle_wrap.h
+++ b/src/handle_wrap.h
@@ -38,7 +38,15 @@ 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;
+    // 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;
+  }
+
+  static inline bool HasRef(const HandleWrap* wrap) {
+    return wrap != nullptr &&
+           wrap->state_ != kClosed &&
+           uv_has_ref(wrap->GetHandle());
   }
 
   inline uv_handle_t* GetHandle() const { return handle__; }
@@ -56,13 +64,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__;
 };
 
 
diff --git a/src/node.cc b/src/node.cc
index 6d2aad7264548c..e7c8eb177af162 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1726,7 +1726,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);