inspector: make sure timer handles are cleaned up

It is not obvious that timer handles are cleaned up properly
when the current `Environment` exits, nor that the `Environment`
knows to keep track of the closing handles.

This change may not be necessary, because timer handles
close without non-trivial delay (i.e. at the end of the current
event loop term), and JS-based inspector sessions (which are
the only ones we can easily test) are destroyed when cleaning up,
closing the timers as a result. I don’t know what happens
for other kinds of inspector sessions, though.

PR-URL: https://github.com/nodejs/node/pull/26088
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This commit is contained in:
Anna Henningsen 2019-02-14 01:18:10 +01:00
parent e74019fa8b
commit 1ae07acfc7
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9

View File

@ -301,22 +301,30 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel,
class InspectorTimer {
public:
InspectorTimer(uv_loop_t* loop,
InspectorTimer(Environment* env,
double interval_s,
V8InspectorClient::TimerCallback callback,
void* data) : timer_(),
void* data) : env_(env),
callback_(callback),
data_(data) {
uv_timer_init(loop, &timer_);
uv_timer_init(env->event_loop(), &timer_);
int64_t interval_ms = 1000 * interval_s;
uv_timer_start(&timer_, OnTimer, interval_ms, interval_ms);
timer_.data = this;
env->AddCleanupHook(CleanupHook, this);
}
InspectorTimer(const InspectorTimer&) = delete;
void Stop() {
env_->RemoveCleanupHook(CleanupHook, this);
if (timer_.data == this) {
timer_.data = nullptr;
uv_timer_stop(&timer_);
uv_close(reinterpret_cast<uv_handle_t*>(&timer_), TimerClosedCb);
env_->CloseHandle(reinterpret_cast<uv_handle_t*>(&timer_), TimerClosedCb);
}
}
private:
@ -325,6 +333,10 @@ class InspectorTimer {
timer->callback_(timer->data_);
}
static void CleanupHook(void* data) {
static_cast<InspectorTimer*>(data)->Stop();
}
static void TimerClosedCb(uv_handle_t* uvtimer) {
std::unique_ptr<InspectorTimer> timer(
node::ContainerOf(&InspectorTimer::timer_,
@ -334,6 +346,7 @@ class InspectorTimer {
~InspectorTimer() {}
Environment* env_;
uv_timer_t timer_;
V8InspectorClient::TimerCallback callback_;
void* data_;
@ -343,9 +356,9 @@ class InspectorTimer {
class InspectorTimerHandle {
public:
InspectorTimerHandle(uv_loop_t* loop, double interval_s,
InspectorTimerHandle(Environment* env, double interval_s,
V8InspectorClient::TimerCallback callback, void* data) {
timer_ = new InspectorTimer(loop, interval_s, callback, data);
timer_ = new InspectorTimer(env, interval_s, callback, data);
}
InspectorTimerHandle(const InspectorTimerHandle&) = delete;
@ -540,7 +553,7 @@ class NodeInspectorClient : public V8InspectorClient {
TimerCallback callback,
void* data) override {
timers_.emplace(std::piecewise_construct, std::make_tuple(data),
std::make_tuple(env_->event_loop(), interval_s, callback,
std::make_tuple(env_, interval_s, callback,
data));
}