diff --git a/src/async_wrap.h b/src/async_wrap.h index 0a29264189aa98..876bd0c3418c64 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -34,6 +34,7 @@ namespace node { #define NODE_ASYNC_NON_CRYPTO_PROVIDER_TYPES(V) \ V(NONE) \ V(DNSCHANNEL) \ + V(ELDHISTOGRAM) \ V(FILEHANDLE) \ V(FILEHANDLECLOSEREQ) \ V(FSEVENTWRAP) \ diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index 0959b7ab130036..8ed6b2948dffd9 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -84,6 +84,17 @@ void HandleWrap::Close(Local close_callback) { } +void HandleWrap::MakeWeak() { + persistent().SetWeak( + this, + [](const v8::WeakCallbackInfo& data) { + HandleWrap* handle_wrap = data.GetParameter(); + handle_wrap->persistent().Reset(); + handle_wrap->Close(); + }, v8::WeakCallbackType::kParameter); +} + + void HandleWrap::MarkAsInitialized() { env()->handle_wrap_queue()->PushBack(this); state_ = kInitialized; @@ -115,15 +126,14 @@ void HandleWrap::OnClose(uv_handle_t* handle) { HandleScope scope(env->isolate()); Context::Scope context_scope(env->context()); - // The wrap object should still be there. - CHECK_EQ(wrap->persistent().IsEmpty(), false); CHECK_EQ(wrap->state_, kClosing); wrap->state_ = kClosed; wrap->OnClose(); - if (wrap->object()->Has(env->context(), env->handle_onclose_symbol()) + if (!wrap->persistent().IsEmpty() && + wrap->object()->Has(env->context(), env->handle_onclose_symbol()) .FromMaybe(false)) { wrap->MakeCallback(env->handle_onclose_symbol(), 0, nullptr); } diff --git a/src/handle_wrap.h b/src/handle_wrap.h index b2722511c3c09f..fbcea4ae4487f5 100644 --- a/src/handle_wrap.h +++ b/src/handle_wrap.h @@ -76,6 +76,8 @@ class HandleWrap : public AsyncWrap { static v8::Local GetConstructorTemplate( Environment* env); + void MakeWeak(); // This hides BaseObject::MakeWeak() + protected: HandleWrap(Environment* env, v8::Local object, diff --git a/src/node_perf.cc b/src/node_perf.cc index da711fee846bb4..0536c39f6c7353 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -477,31 +477,18 @@ static void ELDHistogramNew(const FunctionCallbackInfo& args) { ELDHistogram::ELDHistogram( Environment* env, Local wrap, - int32_t resolution) : BaseObject(env, wrap), + int32_t resolution) : HandleWrap(env, + wrap, + reinterpret_cast(&timer_), + AsyncWrap::PROVIDER_ELDHISTOGRAM), Histogram(1, 3.6e12), resolution_(resolution) { MakeWeak(); - timer_ = new uv_timer_t(); - uv_timer_init(env->event_loop(), timer_); - timer_->data = this; + uv_timer_init(env->event_loop(), &timer_); } -void ELDHistogram::CloseTimer() { - if (timer_ == nullptr) - return; - - env()->CloseHandle(timer_, [](uv_timer_t* handle) { delete handle; }); - timer_ = nullptr; -} - -ELDHistogram::~ELDHistogram() { - Disable(); - CloseTimer(); -} - -void ELDHistogramDelayInterval(uv_timer_t* req) { - ELDHistogram* histogram = - reinterpret_cast(req->data); +void ELDHistogram::DelayIntervalCallback(uv_timer_t* req) { + ELDHistogram* histogram = ContainerOf(&ELDHistogram::timer_, req); histogram->RecordDelta(); TRACE_COUNTER1(TRACING_CATEGORY_NODE2(perf, event_loop), "min", histogram->Min()); @@ -537,21 +524,21 @@ bool ELDHistogram::RecordDelta() { } bool ELDHistogram::Enable() { - if (enabled_) return false; + if (enabled_ || IsHandleClosing()) return false; enabled_ = true; prev_ = 0; - uv_timer_start(timer_, - ELDHistogramDelayInterval, + uv_timer_start(&timer_, + DelayIntervalCallback, resolution_, resolution_); - uv_unref(reinterpret_cast(timer_)); + uv_unref(reinterpret_cast(&timer_)); return true; } bool ELDHistogram::Disable() { - if (!enabled_) return false; + if (!enabled_ || IsHandleClosing()) return false; enabled_ = false; - uv_timer_stop(timer_); + uv_timer_stop(&timer_); return true; } diff --git a/src/node_perf.h b/src/node_perf.h index 4c7585d65ff276..e8441e3bb72444 100644 --- a/src/node_perf.h +++ b/src/node_perf.h @@ -123,14 +123,12 @@ class GCPerformanceEntry : public PerformanceEntry { PerformanceGCKind gckind_; }; -class ELDHistogram : public BaseObject, public Histogram { +class ELDHistogram : public HandleWrap, public Histogram { public: ELDHistogram(Environment* env, Local wrap, int32_t resolution); - ~ELDHistogram() override; - bool RecordDelta(); bool Enable(); bool Disable(); @@ -149,13 +147,13 @@ class ELDHistogram : public BaseObject, public Histogram { SET_SELF_SIZE(ELDHistogram) private: - void CloseTimer(); + static void DelayIntervalCallback(uv_timer_t* req); bool enabled_ = false; int32_t resolution_ = 0; int64_t exceeds_ = 0; uint64_t prev_ = 0; - uv_timer_t* timer_; + uv_timer_t timer_; }; } // namespace performance diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 13aee51175f618..f2702b4b497597 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -50,6 +50,7 @@ const { getSystemErrorName } = require('util'); delete providers.KEYPAIRGENREQUEST; delete providers.HTTPCLIENTREQUEST; delete providers.HTTPINCOMINGMESSAGE; + delete providers.ELDHISTOGRAM; const objKeys = Object.keys(providers); if (objKeys.length > 0)