From 0f97d6066a9037f591d9b3f4f220448ec5c89f29 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 3 Jul 2020 09:42:46 -0700 Subject: [PATCH] quic: use TimerWrap for idle and retransmit timers Signed-off-by: James M Snell PR-URL: https://github.com/nodejs/node/pull/34186 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina --- src/quic/node_quic_session-inl.h | 9 +++---- src/quic/node_quic_session.cc | 7 +++--- src/quic/node_quic_session.h | 5 ++-- src/quic/node_quic_util-inl.h | 43 -------------------------------- src/quic/node_quic_util.h | 32 ------------------------ 5 files changed, 9 insertions(+), 87 deletions(-) diff --git a/src/quic/node_quic_session-inl.h b/src/quic/node_quic_session-inl.h index 935c8d024d55c9..94f84c5c793edf 100644 --- a/src/quic/node_quic_session-inl.h +++ b/src/quic/node_quic_session-inl.h @@ -375,8 +375,7 @@ QuicCID QuicSession::dcid() const { // timer to actually monitor. Here we take the calculated timeout // and extend out the libuv timer. void QuicSession::UpdateRetransmitTimer(uint64_t timeout) { - DCHECK_NOT_NULL(retransmit_); - retransmit_->Update(timeout); + retransmit_.Update(timeout, timeout); } void QuicSession::CheckAllocatedSize(size_t previous_size) const { @@ -512,13 +511,11 @@ void QuicSession::set_remote_transport_params() { } void QuicSession::StopIdleTimer() { - CHECK_NOT_NULL(idle_); - idle_->Stop(); + idle_.Stop(); } void QuicSession::StopRetransmitTimer() { - CHECK_NOT_NULL(retransmit_); - retransmit_->Stop(); + retransmit_.Stop(); } // Called by the OnVersionNegotiation callback when a version diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index fe30411d0def31..75a1cc2b7f77b2 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -1431,8 +1431,8 @@ QuicSession::QuicSession( socket_(socket), alpn_(alpn), hostname_(hostname), - idle_(new Timer(socket->env(), [this]() { OnIdleTimeout(); })), - retransmit_(new Timer(socket->env(), [this]() { MaybeTimeout(); })), + idle_(socket->env(), [this](void* data) { OnIdleTimeout(); }), + retransmit_(socket->env(), [this](void* data) { MaybeTimeout(); }), dcid_(dcid), state_(env()->isolate()), quic_state_(socket->quic_state()) { @@ -2461,14 +2461,13 @@ void QuicSession::UpdateConnectionID( // will be silently closed. It is important to update this as activity // occurs to keep the idle timer from firing. void QuicSession::UpdateIdleTimer() { - CHECK_NOT_NULL(idle_); uint64_t now = uv_hrtime(); uint64_t expiry = ngtcp2_conn_get_idle_expiry(connection()); // nano to millis uint64_t timeout = expiry > now ? (expiry - now) / 1000000ULL : 1; if (timeout == 0) timeout = 1; Debug(this, "Updating idle timeout to %" PRIu64, timeout); - idle_->Update(timeout); + idle_.Update(timeout, timeout); } diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index 9cbaa317ae5876..bac8bcd1b9a1a9 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -18,6 +18,7 @@ #include "node_quic_util.h" #include "node_sockaddr.h" #include "stream_base.h" +#include "timer_wrap.h" #include "v8.h" #include "uv.h" @@ -1471,8 +1472,8 @@ class QuicSession : public AsyncWrap, QuicSessionListener* listener_ = nullptr; JSQuicSessionListener default_listener_; - TimerPointer idle_; - TimerPointer retransmit_; + TimerWrapHandle idle_; + TimerWrapHandle retransmit_; QuicCID scid_; QuicCID dcid_; diff --git a/src/quic/node_quic_util-inl.h b/src/quic/node_quic_util-inl.h index d51dc8d7e63d23..65b2e08823da54 100644 --- a/src/quic/node_quic_util-inl.h +++ b/src/quic/node_quic_util-inl.h @@ -73,49 +73,6 @@ size_t GetMaxPktLen(const SocketAddress& addr) { NGTCP2_MAX_PKTLEN_IPV4; } -Timer::Timer(Environment* env, std::function fn) - : env_(env), - fn_(fn) { - uv_timer_init(env_->event_loop(), &timer_); - timer_.data = this; -} - -void Timer::Stop() { - if (stopped_) - return; - stopped_ = true; - - if (timer_.data == this) { - uv_timer_stop(&timer_); - timer_.data = nullptr; - } -} - -// If the timer is not currently active, interval must be either 0 or greater. -// If the timer is already active, interval is ignored. -void Timer::Update(uint64_t interval) { - if (stopped_) - return; - uv_timer_start(&timer_, OnTimeout, interval, interval); - uv_unref(reinterpret_cast(&timer_)); -} - -void Timer::Free(Timer* timer) { - timer->env_->CloseHandle( - reinterpret_cast(&timer->timer_), - [&](uv_handle_t* timer) { - Timer* t = ContainerOf( - &Timer::timer_, - reinterpret_cast(timer)); - delete t; - }); -} - -void Timer::OnTimeout(uv_timer_t* timer) { - Timer* t = ContainerOf(&Timer::timer_, timer); - t->fn_(); -} - QuicError::QuicError( int32_t family_, uint64_t code_) : diff --git a/src/quic/node_quic_util.h b/src/quic/node_quic_util.h index d698a6df0b4041..b3afcb9483d940 100644 --- a/src/quic/node_quic_util.h +++ b/src/quic/node_quic_util.h @@ -338,38 +338,6 @@ class QuicCID : public MemoryRetainer { const ngtcp2_cid* ptr_; }; -// Simple timer wrapper that is used to implement the internals -// for idle and retransmission timeouts. Call Update to start or -// reset the timer; Stop to halt the timer. -class Timer final : public MemoryRetainer { - public: - inline explicit Timer(Environment* env, std::function fn); - - // Stops the timer with the side effect of the timer no longer being usable. - // It will be cleaned up and the Timer object will be destroyed. - inline void Stop(); - - // If the timer is not currently active, interval must be either 0 or greater. - // If the timer is already active, interval is ignored. - inline void Update(uint64_t interval); - - static inline void Free(Timer* timer); - - SET_NO_MEMORY_INFO() - SET_MEMORY_INFO_NAME(Timer) - SET_SELF_SIZE(Timer) - - private: - static inline void OnTimeout(uv_timer_t* timer); - - bool stopped_ = false; - Environment* env_; - std::function fn_; - uv_timer_t timer_; -}; - -using TimerPointer = DeleteFnPtr; - // A Stateless Reset Token is a mechanism by which a QUIC // endpoint can discreetly signal to a peer that it has // lost all state associated with a connection. This