diff options
author | Anna Henningsen <anna@addaleax.net> | 2020-03-27 23:38:39 +0300 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2020-04-10 18:47:11 +0300 |
commit | 583edd9526b3b9a6965761c7cf53495957f082ef (patch) | |
tree | 99919cdad8eb2f0a46402fc5a2ffd7b987d66e17 /src/inspector_agent.cc | |
parent | c66421452686c1f93a708b1edcd4ca8d48037f25 (diff) |
src: fix cleanup hook removal for InspectorTimer
Fix this to account for the fact that `Stop()` may already have been
called from a cleanup hook when the `inspector::Agent` is deleted
along with the `Environment`, at which point cleanup hooks are no
longer available.
PR-URL: https://github.com/nodejs/node/pull/32523
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'src/inspector_agent.cc')
-rw-r--r-- | src/inspector_agent.cc | 40 |
1 files changed, 24 insertions, 16 deletions
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index b7ccf641c29..2852a0aefbe 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -357,32 +357,26 @@ class InspectorTimer { 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 == nullptr) return; - if (timer_.data == this) { - timer_.data = nullptr; - uv_timer_stop(&timer_); - env_->CloseHandle(reinterpret_cast<uv_handle_t*>(&timer_), TimerClosedCb); - } + timer_.data = nullptr; + uv_timer_stop(&timer_); + env_->CloseHandle(reinterpret_cast<uv_handle_t*>(&timer_), TimerClosedCb); } + inline Environment* env() const { return env_; } + private: static void OnTimer(uv_timer_t* uvtimer) { InspectorTimer* timer = node::ContainerOf(&InspectorTimer::timer_, uvtimer); 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_, @@ -405,16 +399,29 @@ class InspectorTimerHandle { InspectorTimerHandle(Environment* env, double interval_s, V8InspectorClient::TimerCallback callback, void* data) { timer_ = new InspectorTimer(env, interval_s, callback, data); + + env->AddCleanupHook(CleanupHook, this); } InspectorTimerHandle(const InspectorTimerHandle&) = delete; ~InspectorTimerHandle() { - CHECK_NOT_NULL(timer_); - timer_->Stop(); - timer_ = nullptr; + Stop(); } + private: + void Stop() { + if (timer_ != nullptr) { + timer_->env()->RemoveCleanupHook(CleanupHook, this); + timer_->Stop(); + } + timer_ = nullptr; + } + + static void CleanupHook(void* data) { + static_cast<InspectorTimerHandle*>(data)->Stop(); + } + InspectorTimer* timer_; }; @@ -737,8 +744,9 @@ class NodeInspectorClient : public V8InspectorClient { bool is_main_; bool running_nested_loop_ = false; std::unique_ptr<V8Inspector> client_; - std::unordered_map<int, std::unique_ptr<ChannelImpl>> channels_; + // Note: ~ChannelImpl may access timers_ so timers_ has to come first. std::unordered_map<void*, InspectorTimerHandle> timers_; + std::unordered_map<int, std::unique_ptr<ChannelImpl>> channels_; int next_session_id_ = 1; bool waiting_for_resume_ = false; bool waiting_for_frontend_ = false; |