From 685b9b2a6a1fd4d9cec75c1c767c1e8ca2a1baf4 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 6 Jun 2018 16:29:30 -0400 Subject: [PATCH] src: do not persist timer handle in cares_wrap Instead of relying on garbage collection to close the timer handle, manage its state more explicitly. PR-URL: https://github.com/nodejs/node/pull/21093 Fixes: https://github.com/nodejs/node/issues/18190 Refs: https://github.com/nodejs/node/pull/18307 Reviewed-By: Jeremiah Senkpiel Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis Reviewed-By: Joyee Cheung Reviewed-By: Colin Ihrig --- src/cares_wrap.cc | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index cc29ddad5586d9..05ef2b7e12090e 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -151,7 +151,8 @@ class ChannelWrap : public AsyncWrap { void Setup(); void EnsureServers(); - void CleanupTimer(); + void StartTimer(); + void CloseTimer(); void ModifyActivityQueryCount(int count); @@ -313,13 +314,7 @@ void ares_sockstate_cb(void* data, if (read || write) { if (!task) { /* New socket */ - - /* If this is the first socket then start the timer. */ - uv_timer_t* timer_handle = channel->timer_handle(); - if (!uv_is_active(reinterpret_cast(timer_handle))) { - CHECK(channel->task_list()->empty()); - uv_timer_start(timer_handle, ChannelWrap::AresTimeout, 1000, 1000); - } + channel->StartTimer(); task = ares_task_create(channel, sock); if (task == nullptr) { @@ -349,7 +344,7 @@ void ares_sockstate_cb(void* data, channel->env()->CloseHandle(&task->poll_watcher, ares_poll_close_cb); if (channel->task_list()->empty()) { - uv_timer_stop(channel->timer_handle()); + channel->CloseTimer(); } } } @@ -490,15 +485,26 @@ void ChannelWrap::Setup() { } library_inited_ = true; +} - /* Initialize the timeout timer. The timer won't be started until the */ - /* first socket is opened. */ - CleanupTimer(); - timer_handle_ = new uv_timer_t(); - timer_handle_->data = static_cast(this); - uv_timer_init(env()->event_loop(), timer_handle_); +void ChannelWrap::StartTimer() { + if (timer_handle_ == nullptr) { + timer_handle_ = new uv_timer_t(); + timer_handle_->data = static_cast(this); + uv_timer_init(env()->event_loop(), timer_handle_); + } else if (uv_is_active(reinterpret_cast(timer_handle_))) { + return; + } + uv_timer_start(timer_handle_, AresTimeout, 1000, 1000); } +void ChannelWrap::CloseTimer() { + if (timer_handle_ == nullptr) + return; + + env()->CloseHandle(timer_handle_, [](uv_timer_t* handle) { delete handle; }); + timer_handle_ = nullptr; +} ChannelWrap::~ChannelWrap() { if (library_inited_) { @@ -508,17 +514,10 @@ ChannelWrap::~ChannelWrap() { } ares_destroy(channel_); - CleanupTimer(); + CloseTimer(); } -void ChannelWrap::CleanupTimer() { - if (timer_handle_ == nullptr) return; - - env()->CloseHandle(timer_handle_, [](uv_timer_t* handle) { delete handle; }); - timer_handle_ = nullptr; -} - void ChannelWrap::ModifyActivityQueryCount(int count) { active_query_count_ += count; if (active_query_count_ < 0) active_query_count_ = 0; @@ -566,6 +565,7 @@ void ChannelWrap::EnsureServers() { /* destroy channel and reset channel */ ares_destroy(channel_); + CloseTimer(); Setup(); }