diff --git a/quic/api/IoBufQuicBatch.cpp b/quic/api/IoBufQuicBatch.cpp index 2f1b415d6..2a24710b6 100644 --- a/quic/api/IoBufQuicBatch.cpp +++ b/quic/api/IoBufQuicBatch.cpp @@ -86,11 +86,10 @@ bool IOBufQuicBatch::flushInternal() { // If error occurred on first socket, kick off second socket immediately if (!written && happyEyeballsState_ && happyEyeballsState_->connAttemptDelayTimeout && - sock_.getEventBase()->isTimeoutScheduled( - happyEyeballsState_->connAttemptDelayTimeout)) { + happyEyeballsState_->connAttemptDelayTimeout + ->isTimerCallbackScheduled()) { + happyEyeballsState_->connAttemptDelayTimeout->cancelTimerCallback(); happyEyeballsState_->connAttemptDelayTimeout->timeoutExpired(); - sock_.getEventBase()->cancelTimeout( - happyEyeballsState_->connAttemptDelayTimeout); } folly::Optional secondSocketErrno; diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index dad96a9be..be0be4963 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -77,13 +77,11 @@ void QuicTransportBase::scheduleTimeout( } void QuicTransportBase::cancelTimeout(QuicTimerCallback* callback) { - if (evb_) { - evb_->cancelTimeout(callback); - } + callback->cancelTimerCallback(); } bool QuicTransportBase::isTimeoutScheduled(QuicTimerCallback* callback) const { - return evb_ ? evb_->isTimeoutScheduled(callback) : false; + return callback->isTimerCallbackScheduled(); } void QuicTransportBase::setPacingTimer( diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index bfb0cfca0..dc06f07c5 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -262,7 +262,7 @@ class TestQuicTransport } std::chrono::milliseconds getLossTimeoutRemainingTime() { - return evb_->getTimeoutTimeRemaining(&lossTimeout_); + return lossTimeout_.getTimerCallbackTimeRemaining(); } void onReadData(const folly::SocketAddress&, ReceivedUdpPacket&& udpPacket) @@ -365,7 +365,7 @@ class TestQuicTransport } void invokeCancelPingTimeout() { - evb_->cancelTimeout(&pingTimeout_); + pingTimeout_.cancelTimerCallback(); } void invokeHandlePingCallbacks() { @@ -377,10 +377,7 @@ class TestQuicTransport } bool isPingTimeoutScheduled() { - if (evb_->isTimeoutScheduled(&pingTimeout_)) { - return true; - } - return false; + return pingTimeout_.isTimerCallbackScheduled(); } auto& writeLooper() { diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 7325d9223..3cb289880 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -2224,14 +2224,15 @@ TEST_F(QuicTransportTest, SendPathChallenge) { EXPECT_FALSE(conn.pendingEvents.schedulePathValidationTimeout); EXPECT_FALSE(conn.outstandingPathValidation); EXPECT_FALSE( - qEvb_->isTimeoutScheduled(&transport_->getPathValidationTimeout())); + transport_->getPathValidationTimeout().isTimerCallbackScheduled()); + loopForWrites(); EXPECT_FALSE(conn.pendingEvents.pathChallenge); EXPECT_TRUE(conn.pendingEvents.schedulePathValidationTimeout); EXPECT_TRUE(conn.outstandingPathValidation); EXPECT_EQ(conn.outstandingPathValidation, pathChallenge); EXPECT_TRUE( - qEvb_->isTimeoutScheduled(&transport_->getPathValidationTimeout())); + transport_->getPathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(1, transport_->getConnectionState().outstandings.packets.size()); auto packet = @@ -2267,18 +2268,18 @@ TEST_F(QuicTransportTest, PathValidationTimeoutExpired) { EXPECT_FALSE(conn.pendingEvents.schedulePathValidationTimeout); EXPECT_FALSE(conn.outstandingPathValidation); EXPECT_FALSE( - qEvb_->isTimeoutScheduled(&transport_->getPathValidationTimeout())); + transport_->getPathValidationTimeout().isTimerCallbackScheduled()); loopForWrites(); EXPECT_FALSE(conn.pendingEvents.pathChallenge); EXPECT_TRUE(conn.pendingEvents.schedulePathValidationTimeout); EXPECT_TRUE(conn.outstandingPathValidation); EXPECT_EQ(conn.outstandingPathValidation, pathChallenge); EXPECT_TRUE( - qEvb_->isTimeoutScheduled(&transport_->getPathValidationTimeout())); + transport_->getPathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(1, transport_->getConnectionState().outstandings.packets.size()); - qEvb_->cancelTimeout(&transport_->getPathValidationTimeout()); + transport_->getPathValidationTimeout().cancelTimerCallback(); transport_->getPathValidationTimeout().timeoutExpired(); EXPECT_FALSE(conn.pendingEvents.schedulePathValidationTimeout); EXPECT_FALSE(conn.outstandingPathValidation); @@ -2303,12 +2304,12 @@ TEST_F(QuicTransportTest, SendPathValidationWhileThereIsOutstandingOne) { EXPECT_TRUE(conn.outstandingPathValidation); EXPECT_EQ(conn.outstandingPathValidation, pathChallenge); EXPECT_TRUE( - qEvb_->isTimeoutScheduled(&transport_->getPathValidationTimeout())); + transport_->getPathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(1, transport_->getConnectionState().outstandings.packets.size()); PathChallengeFrame pathChallenge2(456); - qEvb_->cancelTimeout(&transport_->getPathValidationTimeout()); + transport_->getPathValidationTimeout().cancelTimerCallback(); conn.pendingEvents.schedulePathValidationTimeout = false; conn.outstandingPathValidation = folly::none; conn.pendingEvents.pathChallenge = pathChallenge2; @@ -2321,7 +2322,7 @@ TEST_F(QuicTransportTest, SendPathValidationWhileThereIsOutstandingOne) { EXPECT_TRUE(conn.pendingEvents.schedulePathValidationTimeout); EXPECT_EQ(conn.outstandingPathValidation, pathChallenge2); EXPECT_TRUE( - qEvb_->isTimeoutScheduled(&transport_->getPathValidationTimeout())); + transport_->getPathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(2, transport_->getConnectionState().outstandings.packets.size()); } @@ -2432,7 +2433,7 @@ TEST_F(QuicTransportTest, DoNotResendLostPathChallengeIfNotOutstanding) { ->packet; // Fire path validation timer - qEvb_->cancelTimeout(&transport_->getPathValidationTimeout()); + transport_->getPathValidationTimeout().cancelTimerCallback(); transport_->getPathValidationTimeout().timeoutExpired(); EXPECT_FALSE(conn.pendingEvents.pathChallenge); @@ -4446,41 +4447,41 @@ TEST_F(QuicTransportTest, NoStream) { TEST_F(QuicTransportTest, CancelAckTimeout) { qEvb_->scheduleTimeout(transport_->getAckTimeout(), 1000000ms); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_TRUE(transport_->getAckTimeout()->isTimerCallbackScheduled()); transport_->getConnectionState().pendingEvents.scheduleAckTimeout = false; transport_->onNetworkData( SocketAddress("::1", 10128), NetworkData(IOBuf::copyBuffer("MTA New York Service"), Clock::now())); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_FALSE(transport_->getAckTimeout()->isTimerCallbackScheduled()); } TEST_F(QuicTransportTest, ScheduleAckTimeout) { // Make srtt large so we will use kMinAckTimeout transport_->getConnectionState().lossState.srtt = 25000000us; - EXPECT_FALSE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_FALSE(transport_->getAckTimeout()->isTimerCallbackScheduled()); transport_->getConnectionState().pendingEvents.scheduleAckTimeout = true; transport_->onNetworkData( SocketAddress("::1", 10003), NetworkData( IOBuf::copyBuffer("Never on time, always timeout"), Clock::now())); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_TRUE(transport_->getAckTimeout()->isTimerCallbackScheduled()); EXPECT_NEAR( - qEvb_->getTimeoutTimeRemaining(transport_->getAckTimeout()).count(), + transport_->getAckTimeout()->getTimerCallbackTimeRemaining().count(), 25, 5); } TEST_F(QuicTransportTest, ScheduleAckTimeoutSRTTFactor) { transport_->getConnectionState().lossState.srtt = 50ms; - EXPECT_FALSE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_FALSE(transport_->getAckTimeout()->isTimerCallbackScheduled()); transport_->getConnectionState().pendingEvents.scheduleAckTimeout = true; transport_->onNetworkData( SocketAddress("::1", 10003), NetworkData( IOBuf::copyBuffer("Never on time, always timeout"), Clock::now())); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_TRUE(transport_->getAckTimeout()->isTimerCallbackScheduled()); EXPECT_NEAR( - qEvb_->getTimeoutTimeRemaining(transport_->getAckTimeout()).count(), + transport_->getAckTimeout()->getTimerCallbackTimeRemaining().count(), 50 / 4, 2); } @@ -4491,15 +4492,15 @@ TEST_F(QuicTransportTest, ScheduleAckTimeoutAckFreq) { transport_->getConnectionState() .ackStates.appDataAckState.ackFrequencySequenceNumber = 1; transport_->getConnectionState().ackStates.maxAckDelay = 50ms / 3; - EXPECT_FALSE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_FALSE(transport_->getAckTimeout()->isTimerCallbackScheduled()); transport_->getConnectionState().pendingEvents.scheduleAckTimeout = true; transport_->onNetworkData( SocketAddress("::1", 10003), NetworkData( IOBuf::copyBuffer("Never on time, always timeout"), Clock::now())); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_TRUE(transport_->getAckTimeout()->isTimerCallbackScheduled()); EXPECT_NEAR( - qEvb_->getTimeoutTimeRemaining(transport_->getAckTimeout()).count(), + transport_->getAckTimeout()->getTimerCallbackTimeRemaining().count(), 50 / 3, 2); } @@ -4508,28 +4509,28 @@ TEST_F(QuicTransportTest, ScheduleAckTimeoutFromMaxAckDelay) { // Make srtt large so we will use maxAckDelay transport_->getConnectionState().lossState.srtt = 25000000us; transport_->getConnectionState().ackStates.maxAckDelay = 10ms; - EXPECT_FALSE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_FALSE(transport_->getAckTimeout()->isTimerCallbackScheduled()); transport_->getConnectionState().pendingEvents.scheduleAckTimeout = true; transport_->onNetworkData( SocketAddress("::1", 10003), NetworkData( IOBuf::copyBuffer("Never on time, always timeout"), Clock::now())); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_TRUE(transport_->getAckTimeout()->isTimerCallbackScheduled()); EXPECT_NEAR( - qEvb_->getTimeoutTimeRemaining(transport_->getAckTimeout()).count(), + transport_->getAckTimeout()->getTimerCallbackTimeRemaining().count(), 10, 5); } TEST_F(QuicTransportTest, CloseTransportCancelsAckTimeout) { transport_->getConnectionState().lossState.srtt = 25000000us; - EXPECT_FALSE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_FALSE(transport_->getAckTimeout()->isTimerCallbackScheduled()); transport_->getConnectionState().pendingEvents.scheduleAckTimeout = true; transport_->onNetworkData( SocketAddress("::1", 10003), NetworkData( IOBuf::copyBuffer("Never on time, always timeout"), Clock::now())); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_TRUE(transport_->getAckTimeout()->isTimerCallbackScheduled()); // We need to send some packets, otherwise loss timer won't be scheduled auto stream = transport_->createBidirectionalStream().value(); auto buf = buildRandomInputData(kDefaultUDPSendPacketLen + 20); @@ -4541,7 +4542,7 @@ TEST_F(QuicTransportTest, CloseTransportCancelsAckTimeout) { EXPECT_TRUE(transport_->isLossTimeoutScheduled()); transport_->closeNow(folly::none); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(transport_->getAckTimeout())); + EXPECT_FALSE(transport_->getAckTimeout()->isTimerCallbackScheduled()); EXPECT_FALSE(transport_->isLossTimeoutScheduled()); } @@ -4568,7 +4569,7 @@ TEST_F(QuicTransportTest, IdleTimeoutMin) { transport_->getConnectionState().peerIdleTimeout = 15s; transport_->setIdleTimerNow(); EXPECT_NEAR( - qEvb_->getTimeoutTimeRemaining(&transport_->idleTimeout()).count(), + transport_->idleTimeout().getTimerCallbackTimeRemaining().count(), 15000, 1000); } @@ -4577,16 +4578,16 @@ TEST_F(QuicTransportTest, IdleTimeoutLocalDisabled) { transport_->getConnectionState().transportSettings.idleTimeout = 0s; transport_->getConnectionState().peerIdleTimeout = 15s; transport_->setIdleTimerNow(); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&transport_->idleTimeout())); + EXPECT_FALSE(transport_->idleTimeout().isTimerCallbackScheduled()); } TEST_F(QuicTransportTest, IdleTimeoutPeerDisabled) { transport_->getConnectionState().transportSettings.idleTimeout = 60s; transport_->getConnectionState().peerIdleTimeout = 0s; transport_->setIdleTimerNow(); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&transport_->idleTimeout())); + ASSERT_TRUE(transport_->idleTimeout().isTimerCallbackScheduled()); EXPECT_NEAR( - qEvb_->getTimeoutTimeRemaining(&transport_->idleTimeout()).count(), + transport_->idleTimeout().getTimerCallbackTimeRemaining().count(), 60000, 1000); } diff --git a/quic/client/connector/QuicConnector.cpp b/quic/client/connector/QuicConnector.cpp index 860ad5215..ffc3f9889 100644 --- a/quic/client/connector/QuicConnector.cpp +++ b/quic/client/connector/QuicConnector.cpp @@ -34,7 +34,7 @@ void QuicConnector::onReplaySafe() noexcept { if (cb_) { cb_->onConnectSuccess(); } - qEvb_->cancelTimeout(this); + cancelTimerCallback(); cleanUpAndCloseSocket(); } diff --git a/quic/client/connector/QuicConnector.h b/quic/client/connector/QuicConnector.h index 65aa4b977..62da66ea4 100644 --- a/quic/client/connector/QuicConnector.h +++ b/quic/client/connector/QuicConnector.h @@ -41,9 +41,7 @@ class QuicConnector : private quic::QuicSocket::ConnectionSetupCallback, ~QuicConnector() override { // TODO we shouldn't need to do this but just as a safety measure // to ensure we don't get a callback after detruction. - if (qEvb_) { - qEvb_->cancelTimeout(this); - } + cancelTimerCallback(); } void connect( diff --git a/quic/common/FunctionLooper.cpp b/quic/common/FunctionLooper.cpp index 8e7760a75..28f75e754 100644 --- a/quic/common/FunctionLooper.cpp +++ b/quic/common/FunctionLooper.cpp @@ -51,8 +51,7 @@ void FunctionLooper::commonLoopBody() noexcept { } bool FunctionLooper::schedulePacingTimeout() noexcept { - if (pacingFunc_ && pacingTimer_ && - !pacingTimer_->isTimerCallbackScheduled(this)) { + if (pacingFunc_ && pacingTimer_ && !isTimerCallbackScheduled()) { auto timeUntilWrite = (*pacingFunc_)(); if (timeUntilWrite != 0us) { nextPacingTime_ = Clock::now() + timeUntilWrite; @@ -78,22 +77,21 @@ void FunctionLooper::run(bool thisIteration) noexcept { << " in loop body and using pacing - not rescheduling"; return; } - if (evb_->isLoopCallbackScheduled(this) || - (!fireLoopEarly_ && pacingTimer_ && - pacingTimer_->isTimerCallbackScheduled(this))) { + if (isLoopCallbackScheduled() || + (!fireLoopEarly_ && pacingTimer_ && isTimerCallbackScheduled())) { VLOG(10) << __func__ << ": " << type_ << " already scheduled"; return; } // If we are pacing, we're about to write again, if it's close, just write // now. - if (pacingTimer_ && pacingTimer_->isTimerCallbackScheduled(this)) { + if (pacingTimer_ && isTimerCallbackScheduled()) { auto n = Clock::now(); auto timeUntilWrite = nextPacingTime_ < n ? 0us : std::chrono::duration_cast( nextPacingTime_ - n); if (timeUntilWrite <= 1ms) { - pacingTimer_->cancelTimeout(this); + cancelTimerCallback(); // The next loop is good enough thisIteration = false; } else { @@ -107,10 +105,10 @@ void FunctionLooper::stop() noexcept { VLOG(10) << __func__ << ": " << type_; running_ = false; if (evb_) { - evb_->cancelLoopCallback(this); + cancelLoopCallback(); } if (pacingTimer_) { - pacingTimer_->cancelTimeout(this); + cancelTimerCallback(); } } @@ -119,11 +117,11 @@ bool FunctionLooper::isRunning() const { } bool FunctionLooper::isPacingScheduled() { - return pacingTimer_ && pacingTimer_->isTimerCallbackScheduled(this); + return pacingTimer_ && isTimerCallbackScheduled(); } bool FunctionLooper::isLoopCallbackScheduled() { - return evb_->isLoopCallbackScheduled(this); + return QuicEventBaseLoopCallback::isLoopCallbackScheduled(); } void FunctionLooper::attachEventBase(std::shared_ptr evb) { @@ -138,7 +136,7 @@ void FunctionLooper::detachEventBase() { DCHECK(evb_ && evb_->isInEventBaseThread()); stop(); if (pacingTimer_) { - pacingTimer_->cancelTimeout(this); + cancelTimerCallback(); } evb_ = nullptr; } diff --git a/quic/common/events/FollyQuicEventBase.cpp b/quic/common/events/FollyQuicEventBase.cpp index 51bb7a827..0afbf692b 100644 --- a/quic/common/events/FollyQuicEventBase.cpp +++ b/quic/common/events/FollyQuicEventBase.cpp @@ -13,30 +13,17 @@ FollyQuicEventBase::FollyQuicEventBase(folly::EventBase* evb) { backingEvb_ = evb; } -FollyQuicEventBase::~FollyQuicEventBase() { - loopCallbackWrappers_.clear_and_dispose([](LoopCallbackWrapper* wrapper) { - wrapper->cancelLoopCallback(); - delete wrapper; - }); - timerCallbackWrappers_.clear_and_dispose([](TimerCallbackWrapper* wrapper) { - wrapper->cancelTimeout(); - delete wrapper; - }); -} +FollyQuicEventBase::~FollyQuicEventBase() = default; void FollyQuicEventBase::runInLoop( QuicEventBaseLoopCallback* callback, bool thisIteration) { auto wrapper = static_cast(getImplHandle(callback)); if (!wrapper) { - wrapper = new LoopCallbackWrapper(callback, this); - loopCallbackWrappers_.push_back(*wrapper); + wrapper = new LoopCallbackWrapper(callback); setImplHandle(callback, wrapper); - return backingEvb_->runInLoop(wrapper, thisIteration); - } else { - // This callback is already scheduled. - return; } + return backingEvb_->runInLoop(wrapper, thisIteration); } void FollyQuicEventBase::runInLoop( @@ -100,13 +87,12 @@ bool FollyQuicEventBase::scheduleTimeoutHighRes( } auto wrapper = static_cast(getImplHandle(timerCallback)); - if (wrapper != nullptr) { - // This callback is already scheduled. - return false; + if (wrapper == nullptr) { + // This is the first time this timer callback is getting scheduled. Create a + // wrapper for it. + wrapper = new TimerCallbackWrapper(timerCallback, this); + setImplHandle(timerCallback, wrapper); } - wrapper = new TimerCallbackWrapper(timerCallback, this); - timerCallbackWrappers_.push_back(*wrapper); - setImplHandle(timerCallback, wrapper); return backingEvb_->scheduleTimeoutHighRes(wrapper, timeout); } @@ -119,73 +105,17 @@ void FollyQuicEventBase::scheduleTimeout( } auto wrapper = static_cast(getImplHandle(timerCallback)); - if (wrapper != nullptr) { - // This callback is already scheduled. - return; + if (wrapper == nullptr) { + // This is the first time this timer callback is getting scheduled. Create a + // wrapper for it. + wrapper = new TimerCallbackWrapper(timerCallback, this); + setImplHandle(timerCallback, wrapper); } - wrapper = new TimerCallbackWrapper(timerCallback, this); - timerCallbackWrappers_.push_back(*wrapper); - setImplHandle(timerCallback, wrapper); backingEvb_->timer().scheduleTimeout(wrapper, timeout); } -bool FollyQuicEventBase::isTimeoutScheduled( - QuicTimerCallback* timerCallback) const { - if (!timerCallback || !getImplHandle(timerCallback)) { - // There is no wrapper. Nothing is scheduled. - return false; - } - auto wrapper = - static_cast(getImplHandle(timerCallback)); - return wrapper->isScheduled(); -} - -std::chrono::milliseconds FollyQuicEventBase::getTimeoutTimeRemaining( - QuicTimerCallback* timerCallback) const { - if (!timerCallback || !getImplHandle(timerCallback)) { - // There is no wrapper. Nothing to check. - return std::chrono::milliseconds(0); - } - auto wrapper = - static_cast(getImplHandle(timerCallback)); - return wrapper->getTimeRemaining(); -} - -void FollyQuicEventBase::cancelTimeout(QuicTimerCallback* timerCallback) { - if (!timerCallback || !getImplHandle(timerCallback)) { - // There is no wrapper. Nothing to cancel. - return; - } - auto wrapper = - static_cast(getImplHandle(timerCallback)); - unregisterTimerCallbackInternal(timerCallback); - wrapper->cancelTimeout(); - delete wrapper; -} - std::chrono::milliseconds FollyQuicEventBase::getTimerTickInterval() const { return backingEvb_->timer().getTickInterval(); } -void FollyQuicEventBase::cancelLoopCallback( - QuicEventBaseLoopCallback* callback) { - auto wrapper = static_cast(getImplHandle(callback)); - if (!wrapper) { - return; - } - unregisterLoopCallbackInternal(callback); - wrapper->cancelLoopCallback(); - delete wrapper; -} - -bool FollyQuicEventBase::isLoopCallbackScheduled( - QuicEventBaseLoopCallback* callback) const { - auto wrapper = - static_cast(getImplHandle(callback)); - if (!wrapper) { - return false; - } - return wrapper->isLoopCallbackScheduled(); -} - } // namespace quic diff --git a/quic/common/events/FollyQuicEventBase.h b/quic/common/events/FollyQuicEventBase.h index b14996e26..55b5500ee 100644 --- a/quic/common/events/FollyQuicEventBase.h +++ b/quic/common/events/FollyQuicEventBase.h @@ -11,7 +11,6 @@ #include -#include #include #include @@ -66,42 +65,13 @@ class FollyQuicEventBase : public QuicEventBase { QuicTimerCallback* callback, std::chrono::milliseconds timeout) override; - bool isTimeoutScheduled(QuicTimerCallback* callback) const override; - - std::chrono::milliseconds getTimeoutTimeRemaining( - QuicTimerCallback* callback) const override; - - void cancelTimeout(QuicTimerCallback* callback) override; - [[nodiscard]] std::chrono::milliseconds getTimerTickInterval() const override; - void cancelLoopCallback(QuicEventBaseLoopCallback* callback) override; - - bool isLoopCallbackScheduled( - QuicEventBaseLoopCallback* callback) const override; - folly::EventBase* getBackingEventBase() { return backingEvb_; } private: - void unregisterLoopCallbackInternal(QuicEventBaseLoopCallback* callback) { - auto implHandle = - static_cast(getImplHandle(callback)); - CHECK_NOTNULL(implHandle); - loopCallbackWrappers_.erase(loopCallbackWrappers_.iterator_to(*implHandle)); - setImplHandle(callback, nullptr); - } - - void unregisterTimerCallbackInternal(QuicTimerCallback* callback) { - auto implHandle = - static_cast(getImplHandle(callback)); - CHECK_NOTNULL(implHandle); - timerCallbackWrappers_.erase( - timerCallbackWrappers_.iterator_to(*implHandle)); - setImplHandle(callback, nullptr); - } - class TimerCallbackWrapper : public folly::HHWheelTimer::Callback, public folly::AsyncTimeout, public QuicTimerCallback::TimerCallbackImpl { @@ -110,71 +80,77 @@ class FollyQuicEventBase : public QuicEventBase { QuicTimerCallback* callback, FollyQuicEventBase* evb) : folly::AsyncTimeout(evb->getBackingEventBase()) { - parentEvb_ = evb; callback_ = callback; } friend class FollyQuicEventBase; + // folly::AsyncTimeout and folly::HHWheelTimer::Callback void timeoutExpired() noexcept override { - parentEvb_->unregisterTimerCallbackInternal(callback_); callback_->timeoutExpired(); - delete this; } + // folly::HHWheelTimer::Callback void callbackCanceled() noexcept override { - parentEvb_->unregisterTimerCallbackInternal(callback_); callback_->callbackCanceled(); - delete this; } - bool isScheduled() noexcept { - return folly::AsyncTimeout::isScheduled() || - folly::HHWheelTimer::Callback::isScheduled(); - } - - void cancelTimeout() noexcept { + // QuicTimerCallback::TimerCallbackImpl + void cancelImpl() noexcept override { folly::AsyncTimeout::cancelTimeout(); folly::HHWheelTimer::Callback::cancelTimeout(); } + // QuicTimerCallback::TimerCallbackImpl + [[nodiscard]] bool isScheduledImpl() const noexcept override { + return folly::AsyncTimeout::isScheduled() || + folly::HHWheelTimer::Callback::isScheduled(); + } + + // QuicTimerCallback::TimerCallbackImpl + [[nodiscard]] std::chrono::milliseconds getTimeRemainingImpl() + const noexcept override { + return folly::HHWheelTimer::Callback::getTimeRemaining(); + } + private: - FollyQuicEventBase* parentEvb_; + // Hide these functions + [[nodiscard]] bool isScheduled() const { + return isScheduledImpl(); + } + void cancelTimeout() noexcept { + return cancelImpl(); + } QuicTimerCallback* callback_; - folly::IntrusiveListHook listHook_; }; class LoopCallbackWrapper : public folly::EventBase::LoopCallback, public QuicEventBaseLoopCallback::LoopCallbackImpl { public: - explicit LoopCallbackWrapper( - QuicEventBaseLoopCallback* callback, - FollyQuicEventBase* evb) { - parentEvb_ = evb; + explicit LoopCallbackWrapper(QuicEventBaseLoopCallback* callback) { callback_ = callback; } - friend class FollyQuicEventBase; - + // folly::EventBase::LoopCallback void runLoopCallback() noexcept override { - // We need to remove the callback wrapper from the parent evb's map, call - // the callback, then delete this wrapper. - parentEvb_->unregisterLoopCallbackInternal(callback_); callback_->runLoopCallback(); - delete this; + } + + // QuicEventBaseLoopCallback::LoopCallbackImpl + void cancelImpl() noexcept override { + folly::EventBase::LoopCallback::cancelLoopCallback(); + } + + // QuicEventBaseLoopCallback::LoopCallbackImpl + [[nodiscard]] bool isScheduledImpl() const noexcept override { + return folly::EventBase::LoopCallback::isLoopCallbackScheduled(); } private: - FollyQuicEventBase* parentEvb_; QuicEventBaseLoopCallback* callback_; - folly::IntrusiveListHook listHook_; }; - folly::IntrusiveList - loopCallbackWrappers_; - folly::IntrusiveList - timerCallbackWrappers_; folly::EventBase* backingEvb_{nullptr}; }; diff --git a/quic/common/events/HighResQuicTimer.cpp b/quic/common/events/HighResQuicTimer.cpp index 633f9b347..941c27eda 100644 --- a/quic/common/events/HighResQuicTimer.cpp +++ b/quic/common/events/HighResQuicTimer.cpp @@ -26,43 +26,18 @@ void HighResQuicTimer::scheduleTimeout( // There is no callback. Nothing to schedule. return; } - if (QuicEventBase::getImplHandle(callback)) { - // This callback is already scheduled. - return; + auto wrapper = static_cast( + QuicEventBase::getImplHandle(callback)); + if (wrapper == nullptr) { + // This is the first time this timer callback is getting scheduled. Create a + // wrapper for it. + wrapper = new TimerCallbackWrapper(callback); + QuicEventBase::setImplHandle(callback, wrapper); } - auto* wrapper = new TimerCallbackWrapper(callback, this); - timerCallbackWrappers_.push_back(*wrapper); - QuicEventBase::setImplHandle(callback, wrapper); return wheelTimer_->scheduleTimeout(wrapper, timeout); } - -bool HighResQuicTimer::isTimerCallbackScheduled( - QuicTimerCallback* callback) const { - if (!callback || !QuicEventBase::getImplHandle(callback)) { - // There is no wrapper. Nothing is scheduled. - return false; - } - auto wrapper = static_cast( - QuicEventBase::getImplHandle(callback)); - return wrapper->isScheduled(); -} - -void HighResQuicTimer::cancelTimeout(QuicTimerCallback* callback) { - if (!callback || !QuicEventBase::getImplHandle(callback)) { - // There is no wrapper. Nothing to cancel. - return; - } - auto wrapper = static_cast( - QuicEventBase::getImplHandle(callback)); - wrapper->cancelTimeout(); - unregisterCallbackInternal(callback); - delete wrapper; -} - HighResQuicTimer::~HighResQuicTimer() { - // Resetting the wheel timer cancels all pending timeouts which clears the - // wrappers. + // Resetting the wheel timer cancels all pending timeouts. wheelTimer_.reset(); - CHECK(timerCallbackWrappers_.empty()); } } // namespace quic diff --git a/quic/common/events/HighResQuicTimer.h b/quic/common/events/HighResQuicTimer.h index b10854137..e82eff599 100644 --- a/quic/common/events/HighResQuicTimer.h +++ b/quic/common/events/HighResQuicTimer.h @@ -9,7 +9,6 @@ #include -#include #include #include @@ -28,48 +27,44 @@ class HighResQuicTimer : public QuicTimer { QuicTimerCallback* callback, std::chrono::microseconds timeout) override; - bool isTimerCallbackScheduled(QuicTimerCallback* callback) const override; - - void cancelTimeout(QuicTimerCallback* callback) override; - private: - void unregisterCallbackInternal(QuicTimerCallback* callback) { - QuicEventBase::setImplHandle(callback, nullptr); - } - class TimerCallbackWrapper : public folly::HHWheelTimerHighRes::Callback, - QuicTimerCallback::TimerCallbackImpl { + public QuicTimerCallback::TimerCallbackImpl { public: - explicit TimerCallbackWrapper( - QuicTimerCallback* callback, - HighResQuicTimer* parentTimer) { - parentTimer_ = parentTimer; + explicit TimerCallbackWrapper(QuicTimerCallback* callback) { callback_ = callback; } - friend class HighResQuicTimer; - void timeoutExpired() noexcept override { - parentTimer_->unregisterCallbackInternal(callback_); callback_->timeoutExpired(); - delete this; } void callbackCanceled() noexcept override { - parentTimer_->unregisterCallbackInternal(callback_); callback_->callbackCanceled(); - delete this; + } + + // QuicTimerCallback::TimerCallbackImpl + void cancelImpl() noexcept override { + folly::HHWheelTimerHighRes::Callback::cancelTimeout(); + } + + // QuicTimerCallback::TimerCallbackImpl + [[nodiscard]] bool isScheduledImpl() const noexcept override { + return folly::HHWheelTimerHighRes::Callback::isScheduled(); + } + + // QuicTimerCallback::TimerCallbackImpl + [[nodiscard]] std::chrono::milliseconds getTimeRemainingImpl() + const noexcept override { + // TODO parametrize this type if it's used anywhere outside of tests + return std::chrono::duration_cast( + folly::HHWheelTimerHighRes::Callback::getTimeRemaining()); } private: - HighResQuicTimer* parentTimer_; QuicTimerCallback* callback_; - folly::IntrusiveListHook listHook_; }; - folly::IntrusiveList - timerCallbackWrappers_; - folly::HHWheelTimerHighRes::UniquePtr wheelTimer_; }; diff --git a/quic/common/events/LibevQuicEventBase.cpp b/quic/common/events/LibevQuicEventBase.cpp index 7aff64af5..3f0c7ce89 100644 --- a/quic/common/events/LibevQuicEventBase.cpp +++ b/quic/common/events/LibevQuicEventBase.cpp @@ -11,6 +11,16 @@ namespace { +void libEvTimeoutCallback( + struct ev_loop* /* loop */, + ev_timer* w, + int /* revents */) { + auto wrapper = + static_cast(w->data); + CHECK(wrapper != nullptr); + wrapper->timeoutExpired(); +} + void libEvCheckCallback( struct ev_loop* /* loop */, ev_check* w, @@ -44,7 +54,14 @@ void LibevQuicEventBase::runInLoop( QuicEventBaseLoopCallback* callback, bool /* thisIteration */) { CHECK(isInEventBaseThread()); - loopCallbacks_.push_back(callback); + auto wrapper = static_cast(getImplHandle(callback)); + if (!wrapper) { + wrapper = new LoopCallbackWrapper(callback); + QuicEventBase::setImplHandle(callback, wrapper); + } + if (!wrapper->listHook_.is_linked()) { + loopCallbackWrappers_.push_back(*wrapper); + } } void LibevQuicEventBase::runInEventBaseThreadAndWait( @@ -52,49 +69,53 @@ void LibevQuicEventBase::runInEventBaseThreadAndWait( fn(); } -void LibevQuicEventBase::cancelLoopCallback( - QuicEventBaseLoopCallback* callback) { - auto itr = std::find(loopCallbacks_.begin(), loopCallbacks_.end(), callback); - if (itr != loopCallbacks_.end()) { - loopCallbacks_.erase(itr); - } +void LibevQuicEventBase::scheduleTimeout( + QuicTimerCallback* timerCallback, + std::chrono::milliseconds timeout) { + scheduleTimeout( + timerCallback, + std::chrono::duration_cast(timeout)); } -bool LibevQuicEventBase::isLoopCallbackScheduled( - QuicEventBaseLoopCallback* callback) const { - auto itr = std::find(loopCallbacks_.begin(), loopCallbacks_.end(), callback); - return itr != loopCallbacks_.end(); +bool LibevQuicEventBase::scheduleTimeoutHighRes( + QuicTimerCallback* timerCallback, + std::chrono::microseconds timeout) { + scheduleTimeout(timerCallback, timeout); + return true; } void LibevQuicEventBase::scheduleTimeout( - QuicTimerCallback* callback, - std::chrono::milliseconds timeout) { - CHECK(!isTimeoutScheduled(callback)); - - auto evTimer = new LibevQuicTimer(ev_loop_, true /*selfOwned*/); - - evTimer->scheduleTimeout( - callback, std::chrono::duration_cast(timeout)); - setImplHandle(callback, evTimer); -} - -bool LibevQuicEventBase::isTimeoutScheduled(QuicTimerCallback* callback) const { - auto evTimer = static_cast(getImplHandle(callback)); - return evTimer && evTimer->isTimerCallbackScheduled(callback); -} - -void LibevQuicEventBase::cancelTimeout(QuicTimerCallback* callback) { - auto evTimer = static_cast(getImplHandle(callback)); - if (evTimer) { - evTimer->cancelTimeout(callback); + QuicTimerCallback* timerCallback, + std::chrono::microseconds timeout) { + if (!timerCallback) { + // There is no callback. Nothing to schedule. + return; } + double seconds = timeout.count() / 1000.; + auto wrapper = + static_cast(getImplHandle(timerCallback)); + if (wrapper == nullptr) { + // This is the first time this timer callback is getting scheduled. Create a + // wrapper for it. + wrapper = new TimerCallbackWrapper(timerCallback, ev_loop_); + wrapper->ev_timer_.data = wrapper; + ev_timer_init( + &wrapper->ev_timer_, + libEvTimeoutCallback, + seconds /* after */, + 0. /* repeat */); + setImplHandle(timerCallback, wrapper); + } else { + // We already have a wrapper. Just re-arm it. + ev_timer_set(&wrapper->ev_timer_, seconds /* after */, 0. /* repeat */); + } + + ev_timer_start(ev_loop_, &wrapper->ev_timer_); } void LibevQuicEventBase::checkCallbacks() { - std::vector callbacks; - std::swap(callbacks, loopCallbacks_); - for (auto cb : callbacks) { - cb->runLoopCallback(); + while (!loopCallbackWrappers_.empty()) { + loopCallbackWrappers_.front().runLoopCallback(); } } diff --git a/quic/common/events/LibevQuicEventBase.h b/quic/common/events/LibevQuicEventBase.h index 458535589..e9440bb4d 100644 --- a/quic/common/events/LibevQuicEventBase.h +++ b/quic/common/events/LibevQuicEventBase.h @@ -7,8 +7,8 @@ #pragma once -#include #include +#include #include #include @@ -18,6 +18,7 @@ #include #include +#include namespace quic { /* @@ -25,7 +26,7 @@ namespace quic { * (It is copied from the previous interface in * quic/common/QuicLibevEventBase.h) */ -class LibevQuicEventBase : public QuicEventBase { +class LibevQuicEventBase : public QuicEventBase, public QuicTimer { public: explicit LibevQuicEventBase(struct ev_loop* loop); ~LibevQuicEventBase() override; @@ -42,18 +43,20 @@ class LibevQuicEventBase : public QuicEventBase { bool isInEventBaseThread() const override; - void cancelLoopCallback(QuicEventBaseLoopCallback* /*callback*/) override; - - bool isLoopCallbackScheduled( - QuicEventBaseLoopCallback* /*callback*/) const override; - + // QuicEventBase void scheduleTimeout( QuicTimerCallback* callback, std::chrono::milliseconds timeout) override; - bool isTimeoutScheduled(QuicTimerCallback* callback) const override; + // QuicEventBase + bool scheduleTimeoutHighRes( + QuicTimerCallback* /*callback*/, + std::chrono::microseconds /*timeout*/) override; - void cancelTimeout(QuicTimerCallback* callback) override; + // QuicTimer + void scheduleTimeout( + QuicTimerCallback* callback, + std::chrono::microseconds timeout) override; bool loop() override { return ev_run(ev_loop_, 0); @@ -72,13 +75,6 @@ class LibevQuicEventBase : public QuicEventBase { LOG(FATAL) << __func__ << " not supported in LibevQuicEventBase"; } - bool scheduleTimeoutHighRes( - QuicTimerCallback* /*callback*/, - std::chrono::microseconds /*timeout*/) override { - LOG(FATAL) << __func__ << " not supported in LibevQuicEventBase"; - return false; - } - void runAfterDelay(folly::Function /*cb*/, uint32_t /*milliseconds*/) override { LOG(FATAL) << __func__ << " not supported in LibevQuicEventBase"; @@ -102,24 +98,91 @@ class LibevQuicEventBase : public QuicEventBase { return std::chrono::milliseconds(0); } - std::chrono::milliseconds getTimeoutTimeRemaining( - QuicTimerCallback* /*callback*/) const override { - LOG(FATAL) << __func__ << " not supported in LibevQuicEventBase"; + [[nodiscard]] std::chrono::microseconds getTickInterval() const override { + LOG(WARNING) << __func__ << " is not implemented in LibevQuicEventBase"; + return std::chrono::microseconds(0); } struct ev_loop* getLibevLoop() { return ev_loop_; } + // This is public so the libev callback can access it void checkCallbacks(); - void timeoutExpired(QuicTimerCallback* callback); + // This is public so the libev callback can access it + class TimerCallbackWrapper : public QuicTimerCallback::TimerCallbackImpl { + public: + explicit TimerCallbackWrapper( + QuicTimerCallback* callback, + struct ev_loop* ev_loop) + : callback_(callback), ev_loop_(ev_loop) {} + + friend class LibevQuicEventBase; + + void timeoutExpired() noexcept { + callback_->timeoutExpired(); + } + + void callbackCanceled() noexcept { + callback_->callbackCanceled(); + } + + void cancelImpl() noexcept override { + ev_timer_stop(ev_loop_, &ev_timer_); + } + + [[nodiscard]] bool isScheduledImpl() const noexcept override { + return ev_is_active(&ev_timer_) || ev_is_pending(&ev_timer_); + } + + [[nodiscard]] std::chrono::milliseconds getTimeRemainingImpl() + const noexcept override { + LOG(FATAL) << __func__ << " not implemented in LibevQuicEventBase"; + } + + private: + QuicTimerCallback* callback_; + struct ev_loop* ev_loop_; + ev_timer ev_timer_; + }; private: + class LoopCallbackWrapper + : public QuicEventBaseLoopCallback::LoopCallbackImpl { + public: + explicit LoopCallbackWrapper(QuicEventBaseLoopCallback* callback) + : callback_(callback) {} + + ~LoopCallbackWrapper() override { + listHook_.unlink(); + } + + friend class LibevQuicEventBase; + + void runLoopCallback() noexcept { + listHook_.unlink(); + callback_->runLoopCallback(); + } + + void cancelImpl() noexcept override { + // Removing the callback from the instrusive list is effectively + // cancelling it. + listHook_.unlink(); + } + [[nodiscard]] bool isScheduledImpl() const noexcept override { + return listHook_.is_linked(); + } + + private: + QuicEventBaseLoopCallback* callback_; + folly::IntrusiveListHook listHook_; + }; + struct ev_loop* ev_loop_{EV_DEFAULT}; - // TODO: use an intrusive list instead of a vector for loopCallbacks_ - std::vector loopCallbacks_; + folly::IntrusiveList + loopCallbackWrappers_; ev_check checkWatcher_; std::atomic loopThreadId_; }; diff --git a/quic/common/events/LibevQuicTimer.cpp b/quic/common/events/LibevQuicTimer.cpp deleted file mode 100644 index f0123339a..000000000 --- a/quic/common/events/LibevQuicTimer.cpp +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#include - -namespace { -void libEvTimeoutCallback( - struct ev_loop* /* loop */, - ev_timer* w, - int /* revents */) { - auto quicTimer = static_cast(w->data); - CHECK(quicTimer != nullptr); - quicTimer->timeoutExpired(); -} -} // namespace - -namespace quic { -LibevQuicTimer::LibevQuicTimer(struct ev_loop* libevLoop, bool selfOwned) { - CHECK(libevLoop != nullptr); - ev_loop_ = libevLoop; - selfOwned_ = selfOwned; - timeoutWatcher_.data = this; -} - -LibevQuicTimer::~LibevQuicTimer() { - ev_timer_stop(ev_loop_, &timeoutWatcher_); - callback_ = nullptr; -} - -void LibevQuicTimer::scheduleTimeout( - QuicTimerCallback* callback, - std::chrono::microseconds timeout) { - CHECK(!callback_) - << "Another callback is already scheduled on this QuicTimer"; - callback_ = callback; - QuicEventBase::setImplHandle(callback_, this); - - double seconds = timeout.count() / 10000.; - ev_timer_init( - &timeoutWatcher_, - libEvTimeoutCallback, - seconds /* after */, - 0. /* repeat */); - ev_timer_start(ev_loop_, &timeoutWatcher_); -} - -bool LibevQuicTimer::isTimerCallbackScheduled( - QuicTimerCallback* callback) const { - return callback == callback_; -} - -void LibevQuicTimer::cancelTimeout(QuicTimerCallback* callback) { - CHECK_EQ(callback_, callback); - QuicEventBase::setImplHandle(callback_, nullptr); - ev_timer_stop(ev_loop_, &timeoutWatcher_); - callback_ = nullptr; - if (selfOwned_) { - delete this; - } -} - -void LibevQuicTimer::timeoutExpired() noexcept { - CHECK(callback_); - QuicEventBase::setImplHandle(callback_, nullptr); - callback_->timeoutExpired(); - ev_timer_stop(ev_loop_, &timeoutWatcher_); - callback_ = nullptr; - if (selfOwned_) { - delete this; - } -} -} // namespace quic diff --git a/quic/common/events/LibevQuicTimer.h b/quic/common/events/LibevQuicTimer.h deleted file mode 100644 index dea6471f4..000000000 --- a/quic/common/events/LibevQuicTimer.h +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include -#include - -#include - -namespace quic { - -class LibevQuicTimer : public QuicTimer, - public QuicTimerCallback::TimerCallbackImpl { - public: - explicit LibevQuicTimer(struct ev_loop* libevLoop, bool selfOwned = false); - ~LibevQuicTimer() override; - - void scheduleTimeout( - QuicTimerCallback* callback, - std::chrono::microseconds timeout) override; - - bool isTimerCallbackScheduled(QuicTimerCallback* callback) const override; - - void cancelTimeout(QuicTimerCallback* callback) override; - - [[nodiscard]] std::chrono::microseconds getTickInterval() const override { - LOG(WARNING) << __func__ << " is not implemented in LibevQuicTimer"; - return std::chrono::microseconds(0); - } - - void timeoutExpired() noexcept; - - private: - struct ev_loop* ev_loop_{nullptr}; - ev_timer timeoutWatcher_; - bool selfOwned_{false}; - - QuicTimerCallback* callback_{nullptr}; -}; -} // namespace quic diff --git a/quic/common/events/QuicEventBase.h b/quic/common/events/QuicEventBase.h index daea29157..fdada1d14 100644 --- a/quic/common/events/QuicEventBase.h +++ b/quic/common/events/QuicEventBase.h @@ -20,9 +20,27 @@ class QuicEventBaseLoopCallback { public: friend class QuicEventBase; - virtual ~QuicEventBaseLoopCallback() = default; + virtual ~QuicEventBaseLoopCallback() { + if (implHandle) { + implHandle->cancelImpl(); + delete implHandle; + } + } virtual void runLoopCallback() noexcept = 0; - class LoopCallbackImpl {}; + void cancelLoopCallback() noexcept { + if (implHandle) { + implHandle->cancelImpl(); + } + } + [[nodiscard]] bool isLoopCallbackScheduled() const noexcept { + return implHandle ? implHandle->isScheduledImpl() : false; + } + class LoopCallbackImpl { + public: + virtual ~LoopCallbackImpl() = default; + virtual void cancelImpl() noexcept = 0; + [[nodiscard]] virtual bool isScheduledImpl() const noexcept = 0; + }; private: LoopCallbackImpl* implHandle{nullptr}; @@ -31,7 +49,12 @@ class QuicEventBaseLoopCallback { class QuicTimerCallback { public: friend class QuicEventBase; - virtual ~QuicTimerCallback() = default; + virtual ~QuicTimerCallback() { + if (implHandle) { + implHandle->cancelImpl(); + delete implHandle; + } + } virtual void timeoutExpired() noexcept = 0; /// This callback was canceled. The default implementation is to just /// proxy to `timeoutExpired` but if you care about the difference between @@ -39,7 +62,33 @@ class QuicTimerCallback { virtual void callbackCanceled() noexcept { timeoutExpired(); } - class TimerCallbackImpl {}; + + void cancelTimerCallback() noexcept { + if (implHandle) { + implHandle->cancelImpl(); + } + } + + [[nodiscard]] bool isTimerCallbackScheduled() const noexcept { + return implHandle ? implHandle->isScheduledImpl() : false; + } + + [[nodiscard]] std::chrono::milliseconds getTimerCallbackTimeRemaining() + const noexcept { + if (!implHandle) { + return std::chrono::milliseconds(0); + } + return implHandle->getTimeRemainingImpl(); + } + + class TimerCallbackImpl { + public: + virtual ~TimerCallbackImpl() = default; + virtual void cancelImpl() noexcept = 0; + [[nodiscard]] virtual bool isScheduledImpl() const noexcept = 0; + [[nodiscard]] virtual std::chrono::milliseconds getTimeRemainingImpl() + const noexcept = 0; + }; private: TimerCallbackImpl* implHandle{nullptr}; @@ -82,18 +131,6 @@ class QuicEventBase { QuicTimerCallback* callback, std::chrono::microseconds timeout) = 0; - virtual void cancelLoopCallback(QuicEventBaseLoopCallback* callback) = 0; - - virtual bool isTimeoutScheduled(QuicTimerCallback* callback) const = 0; - - virtual std::chrono::milliseconds getTimeoutTimeRemaining( - QuicTimerCallback* callback) const = 0; - - virtual void cancelTimeout(QuicTimerCallback* callback) = 0; - - virtual bool isLoopCallbackScheduled( - QuicEventBaseLoopCallback* callback) const = 0; - virtual bool loopOnce(int flags = 0) = 0; virtual bool loop() = 0; diff --git a/quic/common/events/QuicTimer.h b/quic/common/events/QuicTimer.h index e55da7f4a..f116a0cbe 100644 --- a/quic/common/events/QuicTimer.h +++ b/quic/common/events/QuicTimer.h @@ -26,10 +26,6 @@ class QuicTimer : public folly::DelayedDestruction { virtual void scheduleTimeout( QuicTimerCallback* callback, std::chrono::microseconds timeout) = 0; - - virtual bool isTimerCallbackScheduled(QuicTimerCallback* callback) const = 0; - - virtual void cancelTimeout(QuicTimerCallback* callback) = 0; }; } // namespace quic diff --git a/quic/common/events/TimerFDQuicTimer.cpp b/quic/common/events/TimerFDQuicTimer.cpp index a70f8dfea..d1138bfbe 100644 --- a/quic/common/events/TimerFDQuicTimer.cpp +++ b/quic/common/events/TimerFDQuicTimer.cpp @@ -27,43 +27,19 @@ void TimerFDQuicTimer::scheduleTimeout( // There is no callback. Nothing to schedule. return; } - if (QuicEventBase::getImplHandle(callback)) { - // This callback is already scheduled. - return; + auto wrapper = static_cast( + QuicEventBase::getImplHandle(callback)); + if (wrapper == nullptr) { + // This is the first time this timer callback is getting scheduled. Create a + // wrapper for it. + wrapper = new TimerCallbackWrapper(callback); + QuicEventBase::setImplHandle(callback, wrapper); } - auto* wrapper = new TimerCallbackWrapper(callback, this); - timerCallbackWrappers_.push_back(*wrapper); - QuicEventBase::setImplHandle(callback, wrapper); return wheelTimer_->scheduleTimeout(wrapper, timeout); } -bool TimerFDQuicTimer::isTimerCallbackScheduled( - QuicTimerCallback* callback) const { - if (!callback || !QuicEventBase::getImplHandle(callback)) { - // There is no wrapper. Nothing is scheduled. - return false; - } - auto wrapper = static_cast( - QuicEventBase::getImplHandle(callback)); - return wrapper->isScheduled(); -} - -void TimerFDQuicTimer::cancelTimeout(QuicTimerCallback* callback) { - if (!callback || !QuicEventBase::getImplHandle(callback)) { - // There is no wrapper. Nothing to cancel. - return; - } - auto wrapper = static_cast( - QuicEventBase::getImplHandle(callback)); - wrapper->cancelTimeout(); - unregisterCallbackInternal(callback); - delete wrapper; -} - TimerFDQuicTimer::~TimerFDQuicTimer() { - // Resetting the wheel timer cancels all pending timeouts which clears the - // wrappers. + // Resetting the wheel timer cancels all pending timeouts. wheelTimer_.reset(); - CHECK(timerCallbackWrappers_.empty()); } } // namespace quic diff --git a/quic/common/events/TimerFDQuicTimer.h b/quic/common/events/TimerFDQuicTimer.h index a3f32bc11..0af5d46d1 100644 --- a/quic/common/events/TimerFDQuicTimer.h +++ b/quic/common/events/TimerFDQuicTimer.h @@ -9,7 +9,6 @@ #include -#include #include #include @@ -28,48 +27,46 @@ class TimerFDQuicTimer : public QuicTimer { QuicTimerCallback* callback, std::chrono::microseconds timeout) override; - bool isTimerCallbackScheduled(QuicTimerCallback* callback) const override; - - void cancelTimeout(QuicTimerCallback* callback) override; - private: - void unregisterCallbackInternal(QuicTimerCallback* callback) { - QuicEventBase::setImplHandle(callback, nullptr); - } - class TimerCallbackWrapper : public folly::HHWheelTimerHighRes::Callback, QuicTimerCallback::TimerCallbackImpl { public: - explicit TimerCallbackWrapper( - QuicTimerCallback* callback, - TimerFDQuicTimer* parentTimer) { - parentTimer_ = parentTimer; + explicit TimerCallbackWrapper(QuicTimerCallback* callback) { callback_ = callback; } friend class TimerFDQuicTimer; void timeoutExpired() noexcept override { - parentTimer_->unregisterCallbackInternal(callback_); callback_->timeoutExpired(); - delete this; } void callbackCanceled() noexcept override { - parentTimer_->unregisterCallbackInternal(callback_); callback_->callbackCanceled(); - delete this; + } + + // QuicTimerCallback::TimerCallbackImpl + void cancelImpl() noexcept override { + folly::HHWheelTimerHighRes::Callback::cancelTimeout(); + } + + // QuicTimerCallback::TimerCallbackImpl + [[nodiscard]] bool isScheduledImpl() const noexcept override { + return folly::HHWheelTimerHighRes::Callback::isScheduled(); + } + + // QuicTimerCallback::TimerCallbackImpl + [[nodiscard]] std::chrono::milliseconds getTimeRemainingImpl() + const noexcept override { + // TODO parametrize this type if it's used anywhere outside of tests + return std::chrono::duration_cast( + folly::HHWheelTimerHighRes::Callback::getTimeRemaining()); } private: - TimerFDQuicTimer* parentTimer_; QuicTimerCallback* callback_; - folly::IntrusiveListHook listHook_; }; - folly::IntrusiveList - timerCallbackWrappers_; - folly::STTimerFDTimeoutManager timeoutMgr_; folly::HHWheelTimerHighRes::UniquePtr wheelTimer_; }; diff --git a/quic/common/test/FunctionLooperTest.cpp b/quic/common/test/FunctionLooperTest.cpp index ad46d07ef..b11720857 100644 --- a/quic/common/test/FunctionLooperTest.cpp +++ b/quic/common/test/FunctionLooperTest.cpp @@ -217,20 +217,20 @@ TEST(FunctionLooperTest, KeepPacing) { EXPECT_EQ(1, count); EXPECT_TRUE(looper->isPacingScheduled()); - pacingTimer->cancelTimeout(looper.get()); + looper->cancelTimerCallback(); EXPECT_FALSE(looper->isPacingScheduled()); looper->timeoutExpired(); EXPECT_EQ(2, count); EXPECT_TRUE(looper->isPacingScheduled()); - pacingTimer->cancelTimeout(looper.get()); + looper->cancelTimerCallback(); EXPECT_FALSE(looper->isPacingScheduled()); looper->timeoutExpired(); EXPECT_EQ(3, count); EXPECT_TRUE(looper->isPacingScheduled()); stopPacing = true; - pacingTimer->cancelTimeout(looper.get()); + looper->cancelTimerCallback(); EXPECT_FALSE(looper->isPacingScheduled()); looper->timeoutExpired(); EXPECT_EQ(4, count); diff --git a/quic/fizz/client/test/QuicClientTransportTest.cpp b/quic/fizz/client/test/QuicClientTransportTest.cpp index dcf843b2b..4b271ac08 100644 --- a/quic/fizz/client/test/QuicClientTransportTest.cpp +++ b/quic/fizz/client/test/QuicClientTransportTest.cpp @@ -1199,8 +1199,8 @@ TEST_F(QuicClientTransportTest, HappyEyeballsWithSingleV4Address) { EXPECT_FALSE(conn.happyEyeballsState.finished); EXPECT_FALSE(conn.peerAddress.isInitialized()); client->start(&clientConnSetupCallback, &clientConnCallback); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_TRUE(conn.happyEyeballsState.finished); EXPECT_EQ(conn.peerAddress, serverAddr); } @@ -1219,8 +1219,8 @@ TEST_F(QuicClientTransportTest, HappyEyeballsWithSingleV6Address) { EXPECT_FALSE(conn.happyEyeballsState.finished); EXPECT_FALSE(conn.peerAddress.isInitialized()); client->start(&clientConnSetupCallback, &clientConnCallback); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_TRUE(conn.happyEyeballsState.finished); EXPECT_EQ(conn.peerAddress, serverAddrV6); } @@ -1231,7 +1231,7 @@ TEST_F(QuicClientTransportTest, IdleTimerResetOnWritingFirstData) { client->start(&clientConnSetupCallback, &clientConnCallback); loopForWrites(); ASSERT_FALSE(client->getConn().receivedNewPacketBeforeWrite); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_TRUE(client->idleTimeout().isTimerCallbackScheduled()); } TEST_F(QuicClientTransportTest, SetQLoggerDcid) { @@ -1393,8 +1393,8 @@ class QuicClientTransportHappyEyeballsTest EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_EQ(socketWrites.size(), 1); EXPECT_TRUE( verifyLongHeader(*socketWrites.at(0), LongHeader::Types::Initial)); @@ -1414,8 +1414,8 @@ class QuicClientTransportHappyEyeballsTest } else { performFakeHandshake(firstAddress); } - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_TRUE(client->getConn().happyEyeballsState.finished); EXPECT_EQ(client->getConn().originalPeerAddress, firstAddress); EXPECT_EQ(client->getConn().peerAddress, firstAddress); @@ -1440,8 +1440,8 @@ class QuicClientTransportHappyEyeballsTest EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_EQ(socketWrites.size(), 1); EXPECT_TRUE( verifyLongHeader(*socketWrites.at(0), LongHeader::Types::Initial)); @@ -1450,11 +1450,11 @@ class QuicClientTransportHappyEyeballsTest // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket @@ -1465,7 +1465,7 @@ class QuicClientTransportHappyEyeballsTest return buf->computeChainDataLength(); })); EXPECT_CALL(*secondSock, write(secondAddress, _)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_EQ(socketWrites.size(), 1); EXPECT_TRUE( @@ -1517,8 +1517,8 @@ class QuicClientTransportHappyEyeballsTest setConnectionIds(); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_EQ(socketWrites.size(), 1); EXPECT_TRUE( @@ -1528,11 +1528,11 @@ class QuicClientTransportHappyEyeballsTest // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket @@ -1543,7 +1543,7 @@ class QuicClientTransportHappyEyeballsTest socketWrites.push_back(buf->clone()); return buf->computeChainDataLength(); })); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_EQ(socketWrites.size(), 1); EXPECT_TRUE( @@ -1591,8 +1591,8 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); EXPECT_TRUE(conn.happyEyeballsState.finished); } @@ -1612,12 +1612,12 @@ class QuicClientTransportHappyEyeballsTest // Continue trying first socket EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToFirstSocket); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_CALL(*sock, write(firstAddress, _)); EXPECT_CALL(*secondSock, write(secondAddress, _)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); } @@ -1638,12 +1638,12 @@ class QuicClientTransportHappyEyeballsTest // Give up first socket EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToFirstSocket); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_CALL(*sock, write(_, _)).Times(0); EXPECT_CALL(*secondSock, write(secondAddress, _)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); } @@ -1674,12 +1674,12 @@ class QuicClientTransportHappyEyeballsTest client->errMessage(cmsgbuf.hdr); EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToFirstSocket); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_CALL(*sock, write(_, _)).Times(0); EXPECT_CALL(*secondSock, write(secondAddress, _)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); #endif } @@ -1694,23 +1694,23 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket EXPECT_CALL(*sock, write(firstAddress, _)) .WillOnce(SetErrnoAndReturn(EAGAIN, -1)); EXPECT_CALL(*secondSock, write(secondAddress, _)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToFirstSocket); @@ -1718,7 +1718,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, write(firstAddress, _)).Times(1); EXPECT_CALL(*secondSock, write(secondAddress, _)).Times(1); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); } @@ -1732,16 +1732,16 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket @@ -1752,7 +1752,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, pauseRead()).Times(2); EXPECT_CALL(*sock, close()).Times(1); EXPECT_CALL(*secondSock, write(secondAddress, _)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToFirstSocket); @@ -1760,7 +1760,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, write(_, _)).Times(0); EXPECT_CALL(*secondSock, write(secondAddress, _)).Times(1); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); } @@ -1775,16 +1775,16 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_CALL(*sock, pauseRead()).Times(2); EXPECT_CALL(*sock, close()).Times(1); @@ -1806,7 +1806,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, write(_, _)).Times(0); EXPECT_CALL(*secondSock, write(secondAddress, _)).Times(1); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); #endif } @@ -1821,23 +1821,23 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket EXPECT_CALL(*sock, write(firstAddress, _)); EXPECT_CALL(*secondSock, write(secondAddress, _)) .WillOnce(SetErrnoAndReturn(EAGAIN, -1)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToFirstSocket); @@ -1845,7 +1845,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, write(firstAddress, _)).Times(1); EXPECT_CALL(*secondSock, write(secondAddress, _)).Times(1); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); } @@ -1859,16 +1859,16 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket @@ -1879,7 +1879,7 @@ class QuicClientTransportHappyEyeballsTest // Socket is paused read for the second time when QuicClientTransport dies EXPECT_CALL(*secondSock, pauseRead()).Times(2); EXPECT_CALL(*secondSock, close()).Times(1); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToFirstSocket); @@ -1887,7 +1887,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, write(firstAddress, _)).Times(1); EXPECT_CALL(*secondSock, write(_, _)).Times(0); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); } @@ -1902,16 +1902,16 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket @@ -1930,7 +1930,7 @@ class QuicClientTransportHappyEyeballsTest // Socket is paused read once during happy eyeballs EXPECT_CALL(*secondSock, pauseRead()).Times(1); EXPECT_CALL(*secondSock, close()).Times(1); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToFirstSocket); @@ -1938,7 +1938,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, write(firstAddress, _)).Times(1); EXPECT_CALL(*secondSock, write(_, _)).Times(0); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); #endif } @@ -1953,16 +1953,16 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket @@ -1970,7 +1970,7 @@ class QuicClientTransportHappyEyeballsTest .WillOnce(SetErrnoAndReturn(EAGAIN, -1)); EXPECT_CALL(*secondSock, write(secondAddress, _)) .WillOnce(SetErrnoAndReturn(EAGAIN, -1)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToFirstSocket); @@ -1978,7 +1978,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*sock, write(firstAddress, _)).Times(1); EXPECT_CALL(*secondSock, write(secondAddress, _)).Times(1); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); } @@ -1992,16 +1992,16 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire loss timeout to trigger write to both first and second // socket @@ -2010,7 +2010,7 @@ class QuicClientTransportHappyEyeballsTest EXPECT_CALL(*secondSock, write(secondAddress, _)) .WillOnce(SetErrnoAndReturn(EBADF, -1)); EXPECT_CALL(clientConnSetupCallback, onConnectionSetupError(_)); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToFirstSocket); @@ -2028,16 +2028,16 @@ class QuicClientTransportHappyEyeballsTest client->start(&clientConnSetupCallback, &clientConnCallback); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Manually expire conn attempt timeout EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); union { struct cmsghdr hdr; @@ -3350,11 +3350,11 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerResetOnRecvNewData) { 0 /* cipherOverhead */, 0 /* largestAcked */)); - qEvb_->cancelTimeout(&client->idleTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + client->idleTimeout().cancelTimerCallback(); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); deliverData(packet->coalesce()); ASSERT_TRUE(client->getConn().receivedNewPacketBeforeWrite); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_TRUE(client->idleTimeout().isTimerCallbackScheduled()); auto packet2 = packetToBuf(createStreamPacket( *serverChosenConnId /* src */, @@ -3364,11 +3364,11 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerResetOnRecvNewData) { *expected, 0 /* cipherOverhead */, 0 /* largestAcked */)); - qEvb_->cancelTimeout(&client->idleTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + client->idleTimeout().cancelTimerCallback(); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); deliverData(packet2->coalesce()); ASSERT_TRUE(client->getConn().receivedNewPacketBeforeWrite); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_TRUE(client->idleTimeout().isTimerCallbackScheduled()); } TEST_F(QuicClientTransportAfterStartTest, IdleTimerNotResetOnDuplicatePacket) { @@ -3388,11 +3388,11 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerNotResetOnDuplicatePacket) { deliverData(packet->coalesce(), false); ASSERT_TRUE(client->getConn().receivedNewPacketBeforeWrite); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_TRUE(client->idleTimeout().isTimerCallbackScheduled()); - qEvb_->cancelTimeout(&client->idleTimeout()); + client->idleTimeout().cancelTimerCallback(); client->getNonConstConn().receivedNewPacketBeforeWrite = false; - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); quicStats_ = std::make_shared>(); client->setTransportStatsCallback(quicStats_); EXPECT_CALL(*quicStats_, onDuplicatedPacketReceived()); @@ -3400,7 +3400,7 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerNotResetOnDuplicatePacket) { deliverData(packet->coalesce(), false); ASSERT_FALSE(client->getConn().receivedNewPacketBeforeWrite); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); client->closeNow(folly::none); } @@ -3427,14 +3427,14 @@ TEST_P(QuicClientTransportAfterStartTestClose, TimeoutsNotSetAfterClose) { } else { client->close(folly::none); } - qEvb_->cancelTimeout(&client->idleTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + client->idleTimeout().cancelTimerCallback(); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); deliverDataWithoutErrorCheck(packet->coalesce()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->lossTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->ackTimeout())); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&client->drainTimeout())); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(client->lossTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(client->ackTimeout().isTimerCallbackScheduled()); + ASSERT_TRUE(client->drainTimeout().isTimerCallbackScheduled()); std::vector indices = getQLogEventIndices(QLogEventType::PacketDrop, qLogger); @@ -3450,13 +3450,13 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerNotResetOnWritingOldData) { // There should still be outstanding packets auto expected = IOBuf::copyBuffer("hello"); - qEvb_->cancelTimeout(&client->idleTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + client->idleTimeout().cancelTimerCallback(); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); client->writeChain(streamId, expected->clone(), false); loopForWrites(); ASSERT_FALSE(client->getConn().receivedNewPacketBeforeWrite); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); client->closeNow(folly::none); } @@ -3477,12 +3477,12 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerResetNoOutstandingPackets) { // Clear out all the outstanding packets to simulate quiescent state. client->getNonConstConn().receivedNewPacketBeforeWrite = false; client->getNonConstConn().outstandings.reset(); - qEvb_->cancelTimeout(&client->idleTimeout()); + client->idleTimeout().cancelTimerCallback(); auto streamId = client->createBidirectionalStream().value(); auto expected = folly::IOBuf::copyBuffer("hello"); client->writeChain(streamId, expected->clone(), false); loopForWrites(); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + ASSERT_TRUE(client->idleTimeout().isTimerCallbackScheduled()); } TEST_F(QuicClientTransportAfterStartTest, IdleTimeoutExpired) { @@ -3490,7 +3490,7 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimeoutExpired) { socketWrites.clear(); client->idleTimeout().timeoutExpired(); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + EXPECT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); EXPECT_TRUE(client->isDraining()); EXPECT_TRUE(client->isClosed()); @@ -4633,7 +4633,7 @@ TEST_F(QuicClientTransportAfterStartTest, WriteThrowsExceptionWhileDraining) { toString(LocalErrorCode::INTERNAL_ERROR).str()); EXPECT_CALL(*sock, write(_, _)).WillRepeatedly(SetErrnoAndReturn(EBADF, -1)); client->close(err); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + EXPECT_FALSE(client->idleTimeout().isTimerCallbackScheduled()); } TEST_F(QuicClientTransportAfterStartTest, DestroyEvbWhileLossTimeoutActive) { @@ -4644,7 +4644,7 @@ TEST_F(QuicClientTransportAfterStartTest, DestroyEvbWhileLossTimeoutActive) { auto write = IOBuf::copyBuffer("no"); client->writeChain(streamId, write->clone(), true); loopForWrites(); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&client->lossTimeout())); + EXPECT_TRUE(client->lossTimeout().isTimerCallbackScheduled()); eventbase_.reset(); } @@ -5383,10 +5383,10 @@ TEST_F( auto& conn = client->getConn(); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - EXPECT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); // Cancel the delay timer because we want to manually fire it - qEvb_->cancelTimeout(&client->happyEyeballsConnAttemptDelayTimeout()); + client->happyEyeballsConnAttemptDelayTimeout().cancelTimerCallback(); auto streamId = client->createBidirectionalStream().value(); client->writeChain(streamId, IOBuf::copyBuffer("hello"), true); @@ -5399,8 +5399,8 @@ TEST_F( EXPECT_FALSE(conn.happyEyeballsState.shouldWriteToSecondSocket); client->happyEyeballsConnAttemptDelayTimeout().timeoutExpired(); EXPECT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); loopForWrites(); // Declared lost EXPECT_FALSE(zeroRttPacketsOutstanding()); @@ -5422,7 +5422,7 @@ TEST_F( socketWrites.push_back(buf->clone()); return buf->computeChainDataLength(); })); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); ASSERT_EQ(socketWrites.size(), 4); EXPECT_TRUE( @@ -5473,8 +5473,8 @@ TEST_F( auto& conn = client->getConn(); EXPECT_EQ(conn.peerAddress, firstAddress); EXPECT_EQ(conn.happyEyeballsState.secondPeerAddress, secondAddress); - ASSERT_TRUE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + ASSERT_TRUE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_CALL(*sock, write(firstAddress, _)) .WillOnce(Invoke( @@ -5486,12 +5486,12 @@ TEST_F( client->writeChain(streamId, IOBuf::copyBuffer("hello"), true); loopForWrites(); EXPECT_FALSE(zeroRttPacketsOutstanding()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + ASSERT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); ASSERT_TRUE(conn.happyEyeballsState.shouldWriteToSecondSocket); - EXPECT_FALSE(qEvb_->isTimeoutScheduled( - &client->happyEyeballsConnAttemptDelayTimeout())); + EXPECT_FALSE(client->happyEyeballsConnAttemptDelayTimeout() + .isTimerCallbackScheduled()); EXPECT_CALL(*secondSock, write(secondAddress, _)) .Times(2) @@ -5500,7 +5500,7 @@ TEST_F( socketWrites.push_back(buf->clone()); return buf->computeChainDataLength(); })); - qEvb_->cancelTimeout(&client->lossTimeout()); + client->lossTimeout().cancelTimerCallback(); client->lossTimeout().timeoutExpired(); ASSERT_EQ(socketWrites.size(), 2); EXPECT_TRUE( diff --git a/quic/fizz/client/test/QuicClientTransportTestUtil.h b/quic/fizz/client/test/QuicClientTransportTestUtil.h index ce13da1d5..3c402e866 100644 --- a/quic/fizz/client/test/QuicClientTransportTestUtil.h +++ b/quic/fizz/client/test/QuicClientTransportTestUtil.h @@ -115,7 +115,7 @@ class TestingQuicClientTransport : public QuicClientTransport { } bool isDraining() { - return evb_->isTimeoutScheduled(&drainTimeout_); + return drainTimeout_.isTimerCallbackScheduled(); } auto& serverInitialParamsSet() { @@ -549,7 +549,7 @@ class QuicClientTransportTestBase : public virtual testing::Test { setUpSocketExpectations(); client->start(&clientConnSetupCallback, &clientConnCallback); setConnectionIds(); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&client->idleTimeout())); + EXPECT_TRUE(client->idleTimeout().isTimerCallbackScheduled()); EXPECT_EQ(socketWrites.size(), 1); EXPECT_TRUE( diff --git a/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp b/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp index 3bf33e9c4..269492c29 100644 --- a/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp +++ b/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp @@ -104,7 +104,7 @@ void startHappyEyeballs( options); } catch (const std::exception&) { // If second socket bind throws exception, give it up - evb->cancelTimeout(&connAttemptDelayTimeout); + connAttemptDelayTimeout.cancelTimerCallback(); connection.happyEyeballsState.finished = true; } } else if (connection.happyEyeballsState.v6PeerAddress.isInitialized()) { @@ -192,7 +192,7 @@ void happyEyeballsOnDataReceived( if (connection.happyEyeballsState.finished) { return; } - socket->getEventBase()->cancelTimeout(&connAttemptDelayTimeout); + connAttemptDelayTimeout.cancelTimerCallback(); connection.happyEyeballsState.finished = true; connection.happyEyeballsState.shouldWriteToFirstSocket = true; connection.happyEyeballsState.shouldWriteToSecondSocket = false; diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index a339d0e10..d63e0dbe2 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -182,11 +182,11 @@ TEST_F(QuicServerTransportTest, IdleTimerResetOnRecvNewData) { 0 /* cipherOverhead */, 0 /* largestAcked */)); - qEvb_->cancelTimeout(&server->idleTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + server->idleTimeout().cancelTimerCallback(); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); recvEncryptedStream(streamId, *expected); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + ASSERT_TRUE(server->idleTimeout().isTimerCallbackScheduled()); + ASSERT_TRUE(server->keepaliveTimeout().isTimerCallbackScheduled()); EXPECT_CALL(*quicStats_, onQuicStreamClosed()); } @@ -196,17 +196,17 @@ TEST_F(QuicServerTransportTest, IdleTimerNotResetOnDuplicatePacket) { auto expected = IOBuf::copyBuffer("hello"); auto packet = recvEncryptedStream(streamId, *expected); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + ASSERT_TRUE(server->idleTimeout().isTimerCallbackScheduled()); + ASSERT_TRUE(server->keepaliveTimeout().isTimerCallbackScheduled()); - qEvb_->cancelTimeout(&server->idleTimeout()); - qEvb_->cancelTimeout(&server->keepaliveTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + server->idleTimeout().cancelTimerCallback(); + server->keepaliveTimeout().cancelTimerCallback(); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->keepaliveTimeout().isTimerCallbackScheduled()); // Try delivering the same packet again deliverData(packet->clone(), false); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->keepaliveTimeout().isTimerCallbackScheduled()); EXPECT_CALL(*quicStats_, onQuicStreamClosed()); } @@ -217,29 +217,29 @@ TEST_F(QuicServerTransportTest, IdleTimerNotResetWhenDataOutstanding) { server->getNonConstConn().receivedNewPacketBeforeWrite = false; StreamId streamId = server->createBidirectionalStream().value(); - qEvb_->cancelTimeout(&server->idleTimeout()); - qEvb_->cancelTimeout(&server->keepaliveTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + server->idleTimeout().cancelTimerCallback(); + server->keepaliveTimeout().cancelTimerCallback(); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); server->writeChain( streamId, IOBuf::copyBuffer("And if the darkness is to keep us apart"), false); loopForWrites(); // It was the first packet - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + EXPECT_TRUE(server->idleTimeout().isTimerCallbackScheduled()); + EXPECT_TRUE(server->keepaliveTimeout().isTimerCallbackScheduled()); // cancel it and write something else. This time idle timer shouldn't set. - qEvb_->cancelTimeout(&server->idleTimeout()); - qEvb_->cancelTimeout(&server->keepaliveTimeout()); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + server->idleTimeout().cancelTimerCallback(); + server->keepaliveTimeout().cancelTimerCallback(); + EXPECT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); server->writeChain( streamId, IOBuf::copyBuffer("And if the daylight feels like it's a long way off"), false); loopForWrites(); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + EXPECT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); + EXPECT_FALSE(server->keepaliveTimeout().isTimerCallbackScheduled()); } TEST_F(QuicServerTransportTest, TimeoutsNotSetAfterClose) { @@ -257,16 +257,16 @@ TEST_F(QuicServerTransportTest, TimeoutsNotSetAfterClose) { server->close(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("how about no"))); - qEvb_->cancelTimeout(&server->idleTimeout()); - qEvb_->cancelTimeout(&server->keepaliveTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + server->idleTimeout().cancelTimerCallback(); + server->keepaliveTimeout().cancelTimerCallback(); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); deliverDataWithoutErrorCheck(packet->clone()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->lossTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->ackTimeout())); - ASSERT_TRUE(qEvb_->isTimeoutScheduled(&server->drainTimeout())); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->keepaliveTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->lossTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->ackTimeout().isTimerCallbackScheduled()); + ASSERT_TRUE(server->drainTimeout().isTimerCallbackScheduled()); } TEST_F(QuicServerTransportTest, InvalidMigrationNoDrain) { @@ -284,22 +284,22 @@ TEST_F(QuicServerTransportTest, InvalidMigrationNoDrain) { server->close(QuicError( QuicErrorCode(TransportErrorCode::INVALID_MIGRATION), std::string("migration disabled"))); - qEvb_->cancelTimeout(&server->idleTimeout()); - qEvb_->cancelTimeout(&server->keepaliveTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + server->idleTimeout().cancelTimerCallback(); + server->keepaliveTimeout().cancelTimerCallback(); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); deliverDataWithoutErrorCheck(packet->clone()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->lossTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->ackTimeout())); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->drainTimeout())); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->keepaliveTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->lossTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->ackTimeout().isTimerCallbackScheduled()); + ASSERT_FALSE(server->drainTimeout().isTimerCallbackScheduled()); } TEST_F(QuicServerTransportTest, IdleTimeoutExpired) { server->idleTimeout().timeoutExpired(); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + EXPECT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); EXPECT_TRUE(server->isDraining()); EXPECT_TRUE(server->isClosed()); auto serverReadCodec = makeClientEncryptedCodec(); @@ -314,14 +314,14 @@ TEST_F(QuicServerTransportTest, KeepaliveTimeoutExpired) { EXPECT_FALSE(server->isDraining()); EXPECT_FALSE(server->isClosed()); - qEvb_->cancelTimeout(&server->idleTimeout()); - qEvb_->cancelTimeout(&server->keepaliveTimeout()); + server->idleTimeout().cancelTimerCallback(); + server->keepaliveTimeout().cancelTimerCallback(); server->getNonConstConn().receivedNewPacketBeforeWrite = true; // After we write, the idletimout and keepalive timeout should be // scheduled and there should be a ping written. loopForWrites(); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + EXPECT_TRUE(server->idleTimeout().isTimerCallbackScheduled()); + EXPECT_TRUE(server->keepaliveTimeout().isTimerCallbackScheduled()); auto serverReadCodec = makeClientEncryptedCodec(); EXPECT_TRUE(verifyFramePresent( serverWrites, *serverReadCodec, QuicFrame::Type::PingFrame)); @@ -330,8 +330,8 @@ TEST_F(QuicServerTransportTest, KeepaliveTimeoutExpired) { TEST_F(QuicServerTransportTest, RecvDataAfterIdleTimeout) { server->idleTimeout().timeoutExpired(); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->keepaliveTimeout())); + EXPECT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); + EXPECT_FALSE(server->keepaliveTimeout().isTimerCallbackScheduled()); EXPECT_TRUE(server->isDraining()); EXPECT_TRUE(server->isClosed()); @@ -1745,8 +1745,8 @@ TEST_F(QuicServerTransportTest, ShortHeaderPacketWithNoFramesAfterClose) { server->close(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("test close"))); - qEvb_->cancelTimeout(&server->idleTimeout()); - ASSERT_FALSE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + server->idleTimeout().cancelTimerCallback(); + ASSERT_FALSE(server->idleTimeout().isTimerCallbackScheduled()); ShortHeader header( ProtectionType::KeyPhaseZero, @@ -1940,7 +1940,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) { EXPECT_FALSE(server->getConn().pendingEvents.pathChallenge); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_TRUE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_TRUE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_TRUE(server->getConn().pathValidationLimiter != nullptr); @@ -1962,7 +1962,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) { deliverData(packetToBuf(packet), false, &newPeer); EXPECT_FALSE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); } TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) { @@ -2008,7 +2008,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) { EXPECT_FALSE(server->getConn().pendingEvents.pathChallenge); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_TRUE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_TRUE(server->pathValidationTimeout().isTimerCallbackScheduled()); ShortHeader header( ProtectionType::KeyPhaseZero, @@ -2028,7 +2028,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) { deliverData(packetToBuf(packet), false, &newPeer); EXPECT_FALSE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_FALSE(server->getConn().writableBytesLimit); // After Pathresponse frame is received, srtt,lrtt = sampleRtt; @@ -2076,7 +2076,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, IgnoreInvalidPathResponse) { EXPECT_FALSE(server->getConn().pendingEvents.pathChallenge); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_TRUE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_TRUE(server->pathValidationTimeout().isTimerCallbackScheduled()); ShortHeader header( ProtectionType::KeyPhaseZero, @@ -2097,7 +2097,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, IgnoreInvalidPathResponse) { deliverData(packetToBuf(packet), false, &newPeer); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_TRUE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_TRUE(server->pathValidationTimeout().isTimerCallbackScheduled()); } TEST_P( @@ -2132,7 +2132,7 @@ TEST_P( EXPECT_FALSE(server->getConn().pendingEvents.pathChallenge); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_TRUE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_TRUE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_TRUE(server->getConn().pathValidationLimiter != nullptr); @@ -2161,7 +2161,7 @@ TEST_P( EXPECT_TRUE(server->isClosed()); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( @@ -2599,7 +2599,7 @@ TEST_F( EXPECT_TRUE(server->getConn().pendingEvents.pathChallenge); EXPECT_FALSE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1); EXPECT_EQ( @@ -2637,7 +2637,7 @@ TEST_F( EXPECT_FALSE(server->getConn().pendingEvents.pathChallenge); EXPECT_FALSE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 0); EXPECT_EQ(server->getConn().lossState.srtt, srtt); @@ -2674,7 +2674,7 @@ TEST_F( EXPECT_EQ(server->getConn().peerAddress, newPeer); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_TRUE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_TRUE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1); EXPECT_EQ( @@ -2711,7 +2711,7 @@ TEST_F( deliverData(std::move(packetData2), false, &newPeer2); EXPECT_FALSE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1); EXPECT_EQ( @@ -2763,7 +2763,7 @@ TEST_F( EXPECT_EQ(server->getConn().peerAddress, newPeer); EXPECT_TRUE(server->getConn().outstandingPathValidation); EXPECT_TRUE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_TRUE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1); EXPECT_EQ( @@ -2799,7 +2799,7 @@ TEST_F( deliverData(std::move(packetData2)); EXPECT_FALSE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); - EXPECT_FALSE(qEvb_->isTimeoutScheduled(&server->pathValidationTimeout())); + EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 0); EXPECT_EQ(server->getConn().lossState.srtt, srtt); diff --git a/quic/server/test/QuicServerTransportTestUtil.h b/quic/server/test/QuicServerTransportTestUtil.h index 447c44765..86dc30c16 100644 --- a/quic/server/test/QuicServerTransportTestUtil.h +++ b/quic/server/test/QuicServerTransportTestUtil.h @@ -88,7 +88,7 @@ class TestingQuicServerTransport : public QuicServerTransport { } bool isDraining() { - return evb_->isTimeoutScheduled(&drainTimeout_); + return drainTimeout_.isTimerCallbackScheduled(); } void triggerCryptoEvent() { @@ -191,7 +191,7 @@ class QuicServerTransportTestBase : public virtual testing::Test { void startTransport() { server->accept(); setupConnection(); - EXPECT_TRUE(qEvb_->isTimeoutScheduled(&server->idleTimeout())); + EXPECT_TRUE(server->idleTimeout().isTimerCallbackScheduled()); EXPECT_EQ(server->getConn().peerConnectionIds.size(), 1); EXPECT_EQ( *server->getConn().clientConnectionId,