Skip to content

Commit c1e903b

Browse files
syszerydjc
authored andcommitted
fix(quinn): handle overdue timers without polling the async timer
drive_timer() used AsyncTimer::poll() to determine whether a protocol deadline had elapsed. Under Tokio's cooperative task budget, Sleep::poll() may return Poll::Pending for an already-expired deadline once the task's budget is exhausted, which can happen when process_conn_events() drains a busy channel. As a result, handle_timeout() is not called even though the deadline has already elapsed. For QUIC, timers such as PTO, loss detection, and idle timeouts are correctness-critical and should not be deferred to a later scheduling round. Fix this by checking runtime.now() >= deadline before consulting the async timer. The clock is not subject to cooperative budgeting. The timer remains responsible only for registering a wakeup when the deadline lies in the future.
1 parent b3b20e1 commit c1e903b

1 file changed

Lines changed: 24 additions & 31 deletions

File tree

quinn/src/connection.rs

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,51 +1157,44 @@ impl State {
11571157
}
11581158
}
11591159

1160-
fn drive_timer(&mut self, cx: &mut Context) -> bool {
1161-
// Check whether we need to (re)set the timer. If so, we must poll again to ensure the
1162-
// timer is registered with the runtime (and check whether it's already
1163-
// expired).
1164-
match self.inner.poll_timeout() {
1165-
Some(deadline) => {
1166-
if let Some(delay) = &mut self.timer {
1167-
// There is no need to reset the tokio timer if the deadline
1168-
// did not change
1169-
if self
1170-
.timer_deadline
1171-
.map(|current_deadline| current_deadline != deadline)
1172-
.unwrap_or(true)
1173-
{
1174-
delay.as_mut().reset(deadline);
1175-
}
1176-
} else {
1177-
self.timer = Some(self.runtime.new_timer(deadline));
1178-
}
1179-
// Store the actual expiration time of the timer
1180-
self.timer_deadline = Some(deadline);
1160+
fn drive_timer(&mut self, cx: &mut Context<'_>) -> bool {
1161+
let Some(deadline) = self.inner.poll_timeout() else {
1162+
self.timer_deadline = None;
1163+
return false;
1164+
};
1165+
1166+
// Use the clock rather than the async timer to detect expiry: Sleep::poll
1167+
// respects Tokio's cooperative budget and can return Pending for elapsed
1168+
// deadlines.
1169+
let now = self.runtime.now();
1170+
if now >= deadline {
1171+
self.inner.handle_timeout(now);
1172+
self.timer_deadline = None;
1173+
return true;
1174+
}
1175+
1176+
match &mut self.timer {
1177+
// Avoid resetting the timer when the deadline is unchanged.
1178+
Some(delay) if self.timer_deadline != Some(deadline) => {
1179+
delay.as_mut().reset(deadline);
11811180
}
11821181
None => {
1183-
self.timer_deadline = None;
1184-
return false;
1182+
self.timer = Some(self.runtime.new_timer(deadline));
11851183
}
1184+
_ => {}
11861185
}
1187-
1188-
if self.timer_deadline.is_none() {
1189-
return false;
1190-
}
1186+
self.timer_deadline = Some(deadline);
11911187

11921188
let delay = self
11931189
.timer
11941190
.as_mut()
11951191
.expect("timer must exist in this state")
11961192
.as_mut();
11971193
if delay.poll(cx).is_pending() {
1198-
// Since there wasn't a timeout event, there is nothing new
1199-
// for the connection to do
12001194
return false;
12011195
}
12021196

1203-
// A timer expired, so the caller needs to check for
1204-
// new transmits, which might cause new timers to be set.
1197+
// The deadline elapsed in the window between the clock check and poll.
12051198
self.inner.handle_timeout(self.runtime.now());
12061199
self.timer_deadline = None;
12071200
true

0 commit comments

Comments
 (0)