Skip to content

Commit

Permalink
src: properly manage timer in cares ChannelWrap
Browse files Browse the repository at this point in the history
This fixes a bug introduced in 727b291
where code managing the `uv_timer_t` for a `ChannelWrap` instance was
left unchanged, when it should have changed the lifetime of the handle
to being tied to the `ChannelWrap` instance’s lifetime.

Fixes: #14599
Ref: #14518
PR-URL: #14634
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
addaleax committed Aug 7, 2017
1 parent daf5596 commit 41a0dfc
Showing 1 changed file with 23 additions and 15 deletions.
38 changes: 23 additions & 15 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ class ChannelWrap : public AsyncWrap {

void Setup();
void EnsureServers();
void CleanupTimer();

inline uv_timer_t* timer_handle() { return &timer_handle_; }
inline uv_timer_t* timer_handle() { return timer_handle_; }
inline ares_channel cares_channel() { return channel_; }
inline bool query_last_ok() const { return query_last_ok_; }
inline void set_query_last_ok(bool ok) { query_last_ok_ = ok; }
Expand All @@ -152,7 +153,7 @@ class ChannelWrap : public AsyncWrap {
static void AresTimeout(uv_timer_t* handle);

private:
uv_timer_t timer_handle_;
uv_timer_t* timer_handle_;
ares_channel channel_;
bool query_last_ok_;
bool is_servers_default_;
Expand All @@ -163,6 +164,7 @@ class ChannelWrap : public AsyncWrap {
ChannelWrap::ChannelWrap(Environment* env,
Local<Object> object)
: AsyncWrap(env, object, PROVIDER_DNSCHANNEL),
timer_handle_(nullptr),
channel_(nullptr),
query_last_ok_(true),
is_servers_default_(true),
Expand Down Expand Up @@ -236,7 +238,8 @@ RB_GENERATE_STATIC(node_ares_task_list, node_ares_task, node, cmp_ares_tasks)
/* This is called once per second by loop->timer. It is used to constantly */
/* call back into c-ares for possibly processing timeouts. */
void ChannelWrap::AresTimeout(uv_timer_t* handle) {
ChannelWrap* channel = ContainerOf(&ChannelWrap::timer_handle_, handle);
ChannelWrap* channel = static_cast<ChannelWrap*>(handle->data);
CHECK_EQ(channel->timer_handle(), handle);
CHECK_EQ(false, RB_EMPTY(channel->task_list()));
ares_process_fd(channel->cares_channel(), ARES_SOCKET_BAD, ARES_SOCKET_BAD);
}
Expand Down Expand Up @@ -505,25 +508,30 @@ void ChannelWrap::Setup() {

/* Initialize the timeout timer. The timer won't be started until the */
/* first socket is opened. */
uv_timer_init(env()->event_loop(), &timer_handle_);
env()->RegisterHandleCleanup(
reinterpret_cast<uv_handle_t*>(&timer_handle_),
[](Environment* env, uv_handle_t* handle, void* arg) {
uv_close(handle, [](uv_handle_t* handle) {
ChannelWrap* channel = ContainerOf(
&ChannelWrap::timer_handle_,
reinterpret_cast<uv_timer_t*>(handle));
channel->env()->FinishHandleCleanup(handle);
});
},
nullptr);
CleanupTimer();
timer_handle_ = new uv_timer_t();
timer_handle_->data = static_cast<void*>(this);
uv_timer_init(env()->event_loop(), timer_handle_);
}


ChannelWrap::~ChannelWrap() {
if (library_inited_)
ares_library_cleanup();

ares_destroy(channel_);
CleanupTimer();
}


void ChannelWrap::CleanupTimer() {
if (timer_handle_ == nullptr) return;

uv_close(reinterpret_cast<uv_handle_t*>(timer_handle_),
[](uv_handle_t* handle) {
delete reinterpret_cast<uv_timer_t*>(handle);
});
timer_handle_ = nullptr;
}


Expand Down

0 comments on commit 41a0dfc

Please sign in to comment.