Skip to content

Commit

Permalink
src: refactor TimerWrap lifetime management
Browse files Browse the repository at this point in the history
Split `Stop(true)` and `Stop(false)` into separate methods since the
actions performed by these are fully distinct.

PR-URL: #34252
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
  • Loading branch information
addaleax committed Jul 14, 2020
1 parent e2f9dc6 commit 874460a
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/quic/node_quic_session-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,11 @@ void QuicSession::set_remote_transport_params() {
}

void QuicSession::StopIdleTimer() {
idle_.Stop();
idle_.Close();
}

void QuicSession::StopRetransmitTimer() {
retransmit_.Stop();
retransmit_.Close();
}

// Called by the OnVersionNegotiation callback when a version
Expand Down
30 changes: 16 additions & 14 deletions src/timer_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ TimerWrap::TimerWrap(Environment* env, const TimerCb& fn)
timer_.data = this;
}

void TimerWrap::Stop(bool close) {
void TimerWrap::Stop() {
if (timer_.data == nullptr) return;
uv_timer_stop(&timer_);
if (LIKELY(close)) {
timer_.data = nullptr;
env_->CloseHandle(reinterpret_cast<uv_handle_t*>(&timer_), TimerClosedCb);
}
}

void TimerWrap::Close() {
timer_.data = nullptr;
env_->CloseHandle(reinterpret_cast<uv_handle_t*>(&timer_), TimerClosedCb);
}

void TimerWrap::TimerClosedCb(uv_handle_t* handle) {
Expand Down Expand Up @@ -54,13 +55,14 @@ TimerWrapHandle::TimerWrapHandle(
env->AddCleanupHook(CleanupHook, this);
}

void TimerWrapHandle::Stop(bool close) {
if (UNLIKELY(!close))
return timer_->Stop(close);
void TimerWrapHandle::Stop() {
return timer_->Stop();
}

void TimerWrapHandle::Close() {
if (timer_ != nullptr) {
timer_->env()->RemoveCleanupHook(CleanupHook, this);
timer_->Stop();
timer_->Close();
}
timer_ = nullptr;
}
Expand All @@ -80,13 +82,13 @@ void TimerWrapHandle::Update(uint64_t interval, uint64_t repeat) {
timer_->Update(interval, repeat);
}

void TimerWrapHandle::CleanupHook(void* data) {
static_cast<TimerWrapHandle*>(data)->Stop();
}

void TimerWrapHandle::MemoryInfo(node::MemoryTracker* tracker) const {
void TimerWrapHandle::MemoryInfo(MemoryTracker* tracker) const {
if (timer_ != nullptr)
tracker->TrackField("timer", *timer_);
}

void TimerWrapHandle::CleanupHook(void* data) {
static_cast<TimerWrapHandle*>(data)->Close();
}

} // namespace node
14 changes: 8 additions & 6 deletions src/timer_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ class TimerWrap final : public MemoryRetainer {

inline Environment* env() const { return env_; }

// Completely stops the timer, making it no longer usable.
void Stop(bool close = true);
// Stop calling the timer callback.
void Stop();
// Render the timer unusable and delete this object.
void Close();

// Starts / Restarts the Timer
void Update(uint64_t interval, uint64_t repeat = 0);

void Ref();

void Unref();

SET_NO_MEMORY_INFO();
Expand All @@ -55,15 +56,15 @@ class TimerWrapHandle : public MemoryRetainer {

TimerWrapHandle(const TimerWrapHandle&) = delete;

~TimerWrapHandle() { Stop(); }
~TimerWrapHandle() { Close(); }

void Update(uint64_t interval, uint64_t repeat = 0);

void Ref();

void Unref();

void Stop(bool close = true);
void Stop();
void Close();

void MemoryInfo(node::MemoryTracker* tracker) const override;

Expand All @@ -72,6 +73,7 @@ class TimerWrapHandle : public MemoryRetainer {

private:
static void CleanupHook(void* data);

TimerWrap* timer_;
};

Expand Down

0 comments on commit 874460a

Please sign in to comment.