Split the alarm logic out of QuicPollEventLoop into its own queue. This should make it simpler to write unit tests that use alarms, since currently, we either have to reach into the underlying class to call Fire() manually, or use quic::simulator to drive the alarms. PiperOrigin-RevId: 706634751
diff --git a/build/source_list.bzl b/build/source_list.bzl index 2294c69..a288134 100644 --- a/build/source_list.bzl +++ b/build/source_list.bzl
@@ -299,6 +299,7 @@ "quic/core/quic_ack_listener_interface.h", "quic/core/quic_alarm.h", "quic/core/quic_alarm_factory.h", + "quic/core/quic_alarm_factory_proxy.h", "quic/core/quic_arena_scoped_ptr.h", "quic/core/quic_bandwidth.h", "quic/core/quic_blocked_writer_interface.h", @@ -349,6 +350,7 @@ "quic/core/quic_path_validator.h", "quic/core/quic_ping_manager.h", "quic/core/quic_process_packet_interface.h", + "quic/core/quic_queue_alarm_factory.h", "quic/core/quic_received_packet_manager.h", "quic/core/quic_sent_packet_manager.h", "quic/core/quic_server_id.h", @@ -668,6 +670,7 @@ "quic/core/quic_packets.cc", "quic/core/quic_path_validator.cc", "quic/core/quic_ping_manager.cc", + "quic/core/quic_queue_alarm_factory.cc", "quic/core/quic_received_packet_manager.cc", "quic/core/quic_sent_packet_manager.cc", "quic/core/quic_server_id.cc",
diff --git a/build/source_list.gni b/build/source_list.gni index 87e9862..52927da 100644 --- a/build/source_list.gni +++ b/build/source_list.gni
@@ -299,6 +299,7 @@ "src/quiche/quic/core/quic_ack_listener_interface.h", "src/quiche/quic/core/quic_alarm.h", "src/quiche/quic/core/quic_alarm_factory.h", + "src/quiche/quic/core/quic_alarm_factory_proxy.h", "src/quiche/quic/core/quic_arena_scoped_ptr.h", "src/quiche/quic/core/quic_bandwidth.h", "src/quiche/quic/core/quic_blocked_writer_interface.h", @@ -349,6 +350,7 @@ "src/quiche/quic/core/quic_path_validator.h", "src/quiche/quic/core/quic_ping_manager.h", "src/quiche/quic/core/quic_process_packet_interface.h", + "src/quiche/quic/core/quic_queue_alarm_factory.h", "src/quiche/quic/core/quic_received_packet_manager.h", "src/quiche/quic/core/quic_sent_packet_manager.h", "src/quiche/quic/core/quic_server_id.h", @@ -668,6 +670,7 @@ "src/quiche/quic/core/quic_packets.cc", "src/quiche/quic/core/quic_path_validator.cc", "src/quiche/quic/core/quic_ping_manager.cc", + "src/quiche/quic/core/quic_queue_alarm_factory.cc", "src/quiche/quic/core/quic_received_packet_manager.cc", "src/quiche/quic/core/quic_sent_packet_manager.cc", "src/quiche/quic/core/quic_server_id.cc",
diff --git a/build/source_list.json b/build/source_list.json index 3b28c42..d1b698f 100644 --- a/build/source_list.json +++ b/build/source_list.json
@@ -298,6 +298,7 @@ "quiche/quic/core/quic_ack_listener_interface.h", "quiche/quic/core/quic_alarm.h", "quiche/quic/core/quic_alarm_factory.h", + "quiche/quic/core/quic_alarm_factory_proxy.h", "quiche/quic/core/quic_arena_scoped_ptr.h", "quiche/quic/core/quic_bandwidth.h", "quiche/quic/core/quic_blocked_writer_interface.h", @@ -348,6 +349,7 @@ "quiche/quic/core/quic_path_validator.h", "quiche/quic/core/quic_ping_manager.h", "quiche/quic/core/quic_process_packet_interface.h", + "quiche/quic/core/quic_queue_alarm_factory.h", "quiche/quic/core/quic_received_packet_manager.h", "quiche/quic/core/quic_sent_packet_manager.h", "quiche/quic/core/quic_server_id.h", @@ -667,6 +669,7 @@ "quiche/quic/core/quic_packets.cc", "quiche/quic/core/quic_path_validator.cc", "quiche/quic/core/quic_ping_manager.cc", + "quiche/quic/core/quic_queue_alarm_factory.cc", "quiche/quic/core/quic_received_packet_manager.cc", "quiche/quic/core/quic_sent_packet_manager.cc", "quiche/quic/core/quic_server_id.cc",
diff --git a/quiche/quic/core/io/quic_poll_event_loop.cc b/quiche/quic/core/io/quic_poll_event_loop.cc index ffe222a..ee9d04d 100644 --- a/quiche/quic/core/io/quic_poll_event_loop.cc +++ b/quiche/quic/core/io/quic_poll_event_loop.cc
@@ -7,15 +7,20 @@ #include <algorithm> #include <cerrno> #include <cmath> +#include <cstddef> #include <memory> +#include <optional> #include <utility> #include <vector> #include "absl/types/span.h" #include "quiche/quic/core/io/quic_event_loop.h" -#include "quiche/quic/core/quic_alarm.h" +#include "quiche/quic/core/io/socket.h" +#include "quiche/quic/core/quic_alarm_factory.h" +#include "quiche/quic/core/quic_alarm_factory_proxy.h" #include "quiche/quic/core/quic_time.h" #include "quiche/quic/platform/api/quic_bug_tracker.h" +#include "quiche/common/platform/api/quiche_logging.h" namespace quic { @@ -78,13 +83,13 @@ void QuicPollEventLoop::RunEventLoopOnce(QuicTime::Delta default_timeout) { const QuicTime start_time = clock_->Now(); - ProcessAlarmsUpTo(start_time); + alarms_.ProcessAlarmsUpTo(start_time); QuicTime::Delta timeout = ComputePollTimeout(start_time, default_timeout); ProcessIoEvents(start_time, timeout); const QuicTime end_time = clock_->Now(); - ProcessAlarmsUpTo(end_time); + alarms_.ProcessAlarmsUpTo(end_time); } QuicTime::Delta QuicPollEventLoop::ComputePollTimeout( @@ -93,10 +98,11 @@ if (has_artificial_events_pending_) { return QuicTime::Delta::Zero(); } - if (alarms_.empty()) { + std::optional<QuicTime> next_alarm = alarms_.GetNextUpcomingAlarm(); + if (!next_alarm.has_value()) { return default_timeout; } - QuicTime end_time = std::min(now + default_timeout, alarms_.begin()->first); + QuicTime end_time = std::min(now + default_timeout, *next_alarm); if (end_time < now) { // We only run a single pass of processing alarm callbacks per // RunEventLoopOnce() call. If an alarm schedules another alarm in the past @@ -203,62 +209,8 @@ ready_list.clear(); } -void QuicPollEventLoop::ProcessAlarmsUpTo(QuicTime time) { - // Determine which alarm callbacks needs to be run. - std::vector<std::weak_ptr<Alarm*>> alarms_to_call; - while (!alarms_.empty() && alarms_.begin()->first <= time) { - auto& [deadline, schedule_handle_weak] = *alarms_.begin(); - alarms_to_call.push_back(std::move(schedule_handle_weak)); - alarms_.erase(alarms_.begin()); - } - // Actually run those callbacks. - for (std::weak_ptr<Alarm*>& schedule_handle_weak : alarms_to_call) { - std::shared_ptr<Alarm*> schedule_handle = schedule_handle_weak.lock(); - if (!schedule_handle) { - // The alarm has been cancelled and might not even exist anymore. - continue; - } - (*schedule_handle)->DoFire(); - } - // Clean up all of the alarms in the front that have been cancelled. - while (!alarms_.empty()) { - if (alarms_.begin()->second.expired()) { - alarms_.erase(alarms_.begin()); - } else { - break; - } - } -} - -QuicAlarm* QuicPollEventLoop::AlarmFactory::CreateAlarm( - QuicAlarm::Delegate* delegate) { - return new Alarm(loop_, QuicArenaScopedPtr<QuicAlarm::Delegate>(delegate)); -} - -QuicArenaScopedPtr<QuicAlarm> QuicPollEventLoop::AlarmFactory::CreateAlarm( - QuicArenaScopedPtr<QuicAlarm::Delegate> delegate, - QuicConnectionArena* arena) { - if (arena != nullptr) { - return arena->New<Alarm>(loop_, std::move(delegate)); - } - return QuicArenaScopedPtr<QuicAlarm>(new Alarm(loop_, std::move(delegate))); -} - -QuicPollEventLoop::Alarm::Alarm( - QuicPollEventLoop* loop, QuicArenaScopedPtr<QuicAlarm::Delegate> delegate) - : QuicAlarm(std::move(delegate)), loop_(loop) {} - -void QuicPollEventLoop::Alarm::SetImpl() { - current_schedule_handle_ = std::make_shared<Alarm*>(this); - loop_->alarms_.insert({deadline(), current_schedule_handle_}); -} - -void QuicPollEventLoop::Alarm::CancelImpl() { - current_schedule_handle_.reset(); -} - std::unique_ptr<QuicAlarmFactory> QuicPollEventLoop::CreateAlarmFactory() { - return std::make_unique<AlarmFactory>(this); + return std::make_unique<QuicAlarmFactoryProxy>(&alarms_); } int QuicPollEventLoop::PollSyscall(pollfd* fds, size_t nfds, int timeout) {
diff --git a/quiche/quic/core/io/quic_poll_event_loop.h b/quiche/quic/core/io/quic_poll_event_loop.h index db29598..ad66c7f 100644 --- a/quiche/quic/core/io/quic_poll_event_loop.h +++ b/quiche/quic/core/io/quic_poll_event_loop.h
@@ -11,15 +11,19 @@ #include <poll.h> #endif +#include <cstddef> #include <memory> +#include <string> +#include <vector> -#include "absl/container/btree_map.h" +#include "absl/base/attributes.h" #include "absl/types/span.h" #include "quiche/quic/core/io/quic_event_loop.h" #include "quiche/quic/core/io/socket.h" -#include "quiche/quic/core/quic_alarm.h" #include "quiche/quic/core/quic_alarm_factory.h" #include "quiche/quic/core/quic_clock.h" +#include "quiche/quic/core/quic_queue_alarm_factory.h" +#include "quiche/quic/core/quic_time.h" #include "quiche/common/quiche_linked_hash_map.h" namespace quic { @@ -71,40 +75,6 @@ QuicSocketEventMask artificially_notify_at_next_iteration = 0; }; - class Alarm : public QuicAlarm { - public: - Alarm(QuicPollEventLoop* loop, - QuicArenaScopedPtr<QuicAlarm::Delegate> delegate); - - void SetImpl() override; - void CancelImpl() override; - - void DoFire() { - current_schedule_handle_.reset(); - Fire(); - } - - private: - QuicPollEventLoop* loop_; - // Deleted when the alarm is cancelled, causing the corresponding weak_ptr - // in the alarm list to not be executed. - std::shared_ptr<Alarm*> current_schedule_handle_; - }; - - class AlarmFactory : public QuicAlarmFactory { - public: - AlarmFactory(QuicPollEventLoop* loop) : loop_(loop) {} - - // QuicAlarmFactory implementation. - QuicAlarm* CreateAlarm(QuicAlarm::Delegate* delegate) override; - QuicArenaScopedPtr<QuicAlarm> CreateAlarm( - QuicArenaScopedPtr<QuicAlarm::Delegate> delegate, - QuicConnectionArena* arena) override; - - private: - QuicPollEventLoop* loop_; - }; - // Used for deferred execution of I/O callbacks. struct ReadyListEntry { SocketFd fd; @@ -117,9 +87,6 @@ // testing things easier. using RegistrationMap = quiche::QuicheLinkedHashMap<SocketFd, std::shared_ptr<Registration>>; - // Alarms are stored as weak pointers, since the alarm can be cancelled and - // disappear while in the queue. - using AlarmList = absl::btree_multimap<QuicTime, std::weak_ptr<Alarm*>>; // Returns the timeout for the next poll(2) call. It is typically the time at // which the next alarm is supposed to activate. @@ -128,10 +95,8 @@ // Calls poll(2) with the provided timeout and dispatches the callbacks // accordingly. void ProcessIoEvents(QuicTime start_time, QuicTime::Delta timeout); - // Calls all of the alarm callbacks that are scheduled before or at |time|. - void ProcessAlarmsUpTo(QuicTime time); - // Adds the I/O callbacks for |fd| to the |ready_lits| as appopriate. + // Adds the I/O callbacks for |fd| to the |ready_lits| as appropriate. void DispatchIoEvent(std::vector<ReadyListEntry>& ready_list, SocketFd fd, short mask); // NOLINT(runtime/int) // Runs all of the callbacks on the ready list. @@ -144,7 +109,7 @@ const QuicClock* clock_; RegistrationMap registrations_; - AlarmList alarms_; + QuicQueueAlarmFactory alarms_; bool has_artificial_events_pending_ = false; };
diff --git a/quiche/quic/core/quic_alarm_factory_proxy.h b/quiche/quic/core/quic_alarm_factory_proxy.h new file mode 100644 index 0000000..2e3ea30 --- /dev/null +++ b/quiche/quic/core/quic_alarm_factory_proxy.h
@@ -0,0 +1,40 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef QUICHE_QUIC_CORE_QUIC_ALARM_FACTORY_PROXY_H_ +#define QUICHE_QUIC_CORE_QUIC_ALARM_FACTORY_PROXY_H_ + +#include <utility> + +#include "absl/base/nullability.h" +#include "quiche/quic/core/quic_alarm.h" +#include "quiche/quic/core/quic_alarm_factory.h" +#include "quiche/quic/core/quic_arena_scoped_ptr.h" +#include "quiche/quic/core/quic_one_block_arena.h" +#include "quiche/common/platform/api/quiche_export.h" + +namespace quic { + +// QuicAlarmFactoryProxy passes calls to the specified unowned QuicAlarmFactory. +class QUICHE_EXPORT QuicAlarmFactoryProxy : public QuicAlarmFactory { + public: + explicit QuicAlarmFactoryProxy(absl::Nonnull<QuicAlarmFactory*> alarm_factory) + : alarm_factory_(*alarm_factory) {} + + QuicAlarm* CreateAlarm(QuicAlarm::Delegate* delegate) override { + return alarm_factory_.CreateAlarm(delegate); + } + QuicArenaScopedPtr<QuicAlarm> CreateAlarm( + QuicArenaScopedPtr<QuicAlarm::Delegate> delegate, + QuicConnectionArena* arena) override { + return alarm_factory_.CreateAlarm(std::move(delegate), arena); + } + + private: + QuicAlarmFactory& alarm_factory_; +}; + +} // namespace quic + +#endif // QUICHE_QUIC_CORE_QUIC_ALARM_FACTORY_PROXY_H_
diff --git a/quiche/quic/core/quic_queue_alarm_factory.cc b/quiche/quic/core/quic_queue_alarm_factory.cc new file mode 100644 index 0000000..8c52b2c --- /dev/null +++ b/quiche/quic/core/quic_queue_alarm_factory.cc
@@ -0,0 +1,79 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "quiche/quic/core/quic_queue_alarm_factory.h" + +#include <memory> +#include <utility> +#include <vector> + +#include "quiche/quic/core/quic_alarm.h" +#include "quiche/quic/core/quic_arena_scoped_ptr.h" +#include "quiche/quic/core/quic_one_block_arena.h" +#include "quiche/quic/core/quic_time.h" + +namespace quic { + +void QuicQueueAlarmFactory::ProcessAlarmsUpTo(QuicTime time) { + // Determine which alarm callbacks needs to be run. + std::vector<std::weak_ptr<Alarm*>> alarms_to_call; + while (!alarms_.empty() && alarms_.begin()->first <= time) { + auto& [deadline, schedule_handle_weak] = *alarms_.begin(); + alarms_to_call.push_back(std::move(schedule_handle_weak)); + alarms_.erase(alarms_.begin()); + } + // Actually run those callbacks. + for (std::weak_ptr<Alarm*>& schedule_handle_weak : alarms_to_call) { + std::shared_ptr<Alarm*> schedule_handle = schedule_handle_weak.lock(); + if (!schedule_handle) { + // The alarm has been cancelled and might not even exist anymore. + continue; + } + (*schedule_handle)->DoFire(); + } + // Clean up all of the alarms in the front that have been cancelled. + while (!alarms_.empty()) { + if (alarms_.begin()->second.expired()) { + alarms_.erase(alarms_.begin()); + } else { + break; + } + } +} + +QuicAlarm* QuicQueueAlarmFactory::CreateAlarm(QuicAlarm::Delegate* delegate) { + return new Alarm(this, QuicArenaScopedPtr<QuicAlarm::Delegate>(delegate)); +} + +QuicArenaScopedPtr<QuicAlarm> QuicQueueAlarmFactory::CreateAlarm( + QuicArenaScopedPtr<QuicAlarm::Delegate> delegate, + QuicConnectionArena* arena) { + if (arena != nullptr) { + return arena->New<Alarm>(this, std::move(delegate)); + } + return QuicArenaScopedPtr<QuicAlarm>(new Alarm(this, std::move(delegate))); +} + +QuicQueueAlarmFactory::Alarm::Alarm( + QuicQueueAlarmFactory* factory, + QuicArenaScopedPtr<QuicAlarm::Delegate> delegate) + : QuicAlarm(std::move(delegate)), factory_(factory) {} + +void QuicQueueAlarmFactory::Alarm::SetImpl() { + current_schedule_handle_ = std::make_shared<Alarm*>(this); + factory_->alarms_.insert({deadline(), current_schedule_handle_}); +} + +void QuicQueueAlarmFactory::Alarm::CancelImpl() { + current_schedule_handle_.reset(); +} + +std::optional<QuicTime> QuicQueueAlarmFactory::GetNextUpcomingAlarm() const { + if (alarms_.empty()) { + return std::nullopt; + } + return alarms_.begin()->first; +} + +} // namespace quic
diff --git a/quiche/quic/core/quic_queue_alarm_factory.h b/quiche/quic/core/quic_queue_alarm_factory.h new file mode 100644 index 0000000..e79607e --- /dev/null +++ b/quiche/quic/core/quic_queue_alarm_factory.h
@@ -0,0 +1,68 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef QUICHE_QUIC_CORE_QUIC_QUEUE_ALARM_FACTORY_H_ +#define QUICHE_QUIC_CORE_QUIC_QUEUE_ALARM_FACTORY_H_ + +#include <memory> +#include <optional> + +#include "absl/container/btree_map.h" +#include "quiche/quic/core/quic_alarm.h" +#include "quiche/quic/core/quic_alarm_factory.h" +#include "quiche/quic/core/quic_arena_scoped_ptr.h" +#include "quiche/quic/core/quic_one_block_arena.h" +#include "quiche/quic/core/quic_time.h" +#include "quiche/common/platform/api/quiche_export.h" + +namespace quic { + +// QuicQueueAlarmFactory maintains a queue of scheduled alarms internally, and +// provides methods to query the time of the next alarm and to execute all of +// the ones that are past the deadline. +class QUICHE_EXPORT QuicQueueAlarmFactory : public QuicAlarmFactory { + public: + // Calls all of the alarm callbacks that are scheduled before or at |time|. + void ProcessAlarmsUpTo(QuicTime time); + + // Returns the deadline of the next upcoming alarm, if any are scheduled. + std::optional<QuicTime> GetNextUpcomingAlarm() const; + + // QuicAlarmFactory implementation. + QuicAlarm* CreateAlarm(QuicAlarm::Delegate* delegate) override; + QuicArenaScopedPtr<QuicAlarm> CreateAlarm( + QuicArenaScopedPtr<QuicAlarm::Delegate> delegate, + QuicConnectionArena* arena) override; + + private: + class Alarm : public QuicAlarm { + public: + Alarm(QuicQueueAlarmFactory* factory, + QuicArenaScopedPtr<QuicAlarm::Delegate> delegate); + + void SetImpl() override; + void CancelImpl() override; + + void DoFire() { + current_schedule_handle_.reset(); + Fire(); + } + + private: + QuicQueueAlarmFactory* factory_; + + // Deleted when the alarm is cancelled, causing the corresponding weak_ptr + // in the alarm list to not be executed. + std::shared_ptr<Alarm*> current_schedule_handle_; + }; + + // Alarms are stored as weak pointers, since the alarm can be cancelled and + // disappear while in the queue. + using AlarmList = absl::btree_multimap<QuicTime, std::weak_ptr<Alarm*>>; + AlarmList alarms_; +}; + +} // namespace quic + +#endif // QUICHE_QUIC_CORE_QUIC_QUEUE_ALARM_FACTORY_H_