diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index 8421d694a9..85c8d9e921 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -20,28 +20,25 @@ using v8::Value; void HandleWrap::Ref(const FunctionCallbackInfo& args) { HandleWrap* wrap = Unwrap(args.Holder()); - if (IsAlive(wrap)) { - uv_ref(wrap->handle__); - wrap->flags_ &= ~kUnref; - } + if (IsAlive(wrap)) + uv_ref(wrap->GetHandle()); } void HandleWrap::Unref(const FunctionCallbackInfo& args) { HandleWrap* wrap = Unwrap(args.Holder()); - if (IsAlive(wrap)) { - uv_unref(wrap->handle__); - wrap->flags_ |= kUnref; - } + if (IsAlive(wrap)) + uv_unref(wrap->GetHandle()); } void HandleWrap::Unrefed(const FunctionCallbackInfo& args) { HandleWrap* wrap = Unwrap(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)); } @@ -50,17 +47,17 @@ void HandleWrap::Close(const FunctionCallbackInfo& args) { HandleWrap* wrap = Unwrap(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; } } @@ -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(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 = 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 d945143d31..fe3c5a8d9d 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& 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&); static void OnClose(uv_handle_t* handle); ListNode 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 2cfb2866b8..479130230b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1723,7 +1723,7 @@ void GetActiveHandles(const FunctionCallbackInfo& args) { Local 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 = w->object(); Local owner = object->Get(owner_sym);