fix(sse): saturate exponential reconnect backoff to avoid overflow panic (#1231)

* fix(sse): saturate exponential reconnect backoff

ExponentialBackoff::retry computed the reconnect multiplier with
2u32.pow(current_times). With max_times unset, current_times can reach
the bit width, panicking in debug builds and wrapping to a zero delay in
release builds for long-lived SSE clients. Use saturating_pow and
Duration::saturating_mul so the delay stays monotonic and panic-free.

* fix(sse): cap exponential reconnect backoff at a bounded max delay

Saturating the multiplier alone can still yield decades-long sleeps once
current_times reaches the bit width, pinning the stream in
tokio::time::sleep without reconnecting or terminating. Add an optional
max_delay (default 30s) that clamps the computed delay, keeping the
backoff monotonic and panic-free while guaranteeing the client retries.

* fix(sse): default max_delay to None to preserve unbounded backoff

Per maintainer feedback, leave ExponentialBackoff::default() unbounded so
the fix stays a pure overflow bug-fix. The saturating multiplier removes
the debug panic / release wrap from #1198, while max_delay stays opt-in
for callers that want a bounded reconnect delay.
This commit is contained in:
ump45nose 2026-09-10 09:24:29 +08:00 • committed by GitHub
commit 46db531df9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 89 additions and 1 deletions

View file

@ -205,6 +205,11 @@ impl Default for FixedInterval {
pub struct ExponentialBackoff {
pub max_times: Option<usize>,
pub base_duration: Duration,
/// Optional upper bound on a single reconnect delay. `None` (the default) preserves the
/// pre-existing unbounded doubling behavior. Once the multiplier saturates near the bit
/// width that can still produce very long sleeps, so callers that need the client to
/// actually reconnect can set `Some(...)` to clamp the delay.
pub max_delay: Option<Duration>,
}
impl ExponentialBackoff {
@ -216,6 +221,7 @@ impl Default for ExponentialBackoff {
Self {
max_times: None,
base_duration: Self::DEFAULT_DURATION,
max_delay: None,
}
}
}
@ -227,7 +233,16 @@ impl SseRetryPolicy for ExponentialBackoff {
{
return None;
}
Some(self.base_duration * (2u32.pow(current_times as u32)))
// `current_times` is unbounded when `max_times` is unset, so the exponent can reach
// the bit width. Saturate the multiplier at `u32::MAX` and use saturating multiplication
// for the base duration so the delay stays monotonic and panic-free instead of an
// overflow panic (debug) or a wrapped-to-zero backoff (release).
let multiplier = 2u32.saturating_pow(current_times as u32);
let delay = self.base_duration.saturating_mul(multiplier);
Some(match self.max_delay {
Some(max_delay) => delay.min(max_delay),
None => delay,
})
}
}
@ -775,4 +790,75 @@ mod tests {
assert!(stream.next().await.is_none());
assert_eq!(attempts.load(Ordering::Relaxed), 0);
}
#[test]
fn exponential_backoff_saturates_at_high_retry_counts() {
// With `max_times` unset, `current_times` can reach the bit width. The old
// `2u32.pow(current_times)` panicked in debug builds and wrapped in release;
// the saturating implementation must return a monotonic, non-zero delay instead.
let policy = ExponentialBackoff {
max_times: None,
base_duration: Duration::from_millis(1),
max_delay: None,
};
let mut previous = Duration::ZERO;
for current_times in [31usize, 32, 63, 64, 100] {
let delay = policy
.retry(current_times)
.expect("unbounded policy never gives up");
assert!(
!delay.is_zero(),
"delay must stay non-zero at {current_times}"
);
assert!(
delay >= previous,
"delay must stay monotonic at {current_times}"
);
previous = delay;
}
}
#[test]
fn exponential_backoff_caps_delay_at_max_delay() {
// An explicit cap keeps the unbounded doubling policy from producing decades-long
// sleeps once the multiplier saturates. The delay must grow monotonically, stop at
// the configured ceiling, and never exceed it.
let policy = ExponentialBackoff {
max_times: None,
base_duration: Duration::from_secs(1),
max_delay: Some(Duration::from_secs(30)),
};
let mut previous = Duration::ZERO;
for current_times in [0usize, 1, 2, 3, 4, 5, 10, 32, 64, 100] {
let delay = policy
.retry(current_times)
.expect("unbounded policy never gives up");
assert!(
delay >= previous,
"delay must stay monotonic at {current_times}"
);
assert!(
delay <= Duration::from_secs(30),
"delay must respect max_delay at {current_times}"
);
previous = delay;
}
// Beyond the ceiling the delay stays pinned at max_delay.
assert_eq!(
policy.retry(100).expect("never gives up"),
Duration::from_secs(30)
);
}
#[test]
fn exponential_backoff_respects_max_times() {
let policy = ExponentialBackoff {
max_times: Some(3),
base_duration: Duration::from_millis(1),
max_delay: None,
};
assert!(policy.retry(0).is_some());
assert!(policy.retry(2).is_some());
assert!(policy.retry(3).is_none());
}
}

View file

@ -2293,6 +2293,7 @@ mod tests {
Arc::new(ExponentialBackoff {
max_times: Some(1),
base_duration: Duration::ZERO,
max_delay: None,
}),
);
let mut stream = std::pin::pin!(stream);
@ -2397,6 +2398,7 @@ mod tests {
Arc::new(ExponentialBackoff {
max_times: Some(1),
base_duration: Duration::ZERO,
max_delay: None,
}),
);