hedge: zero `period` is accepted but panics on first `call` with divide-by-zero

[meng-xu-cs]

Summary

Hedge::new and Hedge::new_with_mock_latencies accept Duration::ZERO for the histogram rotation period, but RotatingHistogram::maybe_rotate unconditionally computes nanos(delta) / nanos(self.period).

That means a zero period is accepted at construction time and then later triggers a divide-by-zero panic on the first histogram access.

For cloneable requests, this can happen during Hedge::call itself: Select::call invokes SelectPolicy::clone_request, which reads the histogram before either inner service is called.

new_with_mock_latencies is also affected: if latencies_ms is non-empty, it can panic during construction because it pre-populates the histogram immediately.

Reproduction

Minimal regression test:

use futures::task::noop_waker;
use std::{
    convert::Infallible,
    future::{ready, Ready},
    panic::{catch_unwind, AssertUnwindSafe},
    sync::{
        atomic::{AtomicBool, Ordering},
        Arc,
    },
    task::{Context, Poll},
    time::Duration,
};
use tower::{
    hedge::{Hedge, Policy},
    Service,
};

#[derive(Clone)]
struct Probe(Arc<AtomicBool>);

impl Service<String> for Probe {
    type Response = String;
    type Error = Infallible;
    type Future = Ready<Result<String, Infallible>>;

    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        Poll::Ready(Ok(()))
    }

    fn call(&mut self, req: String) -> Self::Future {
        self.0.store(true, Ordering::SeqCst);
        ready(Ok(req))
    }
}

#[derive(Clone)]
struct CloneAll;

impl Policy<String> for CloneAll {
    fn clone_request(&self, req: &String) -> Option<String> {
        Some(req.clone())
    }

    fn can_retry(&self, _req: &String) -> bool {
        true
    }
}

#[test]
fn hedge_zero_period_reports_ready_then_panics_on_call() {
    let called = Arc::new(AtomicBool::new(false));
    let mut svc = Hedge::new(Probe(called.clone()), CloneAll, 0, 0.5, Duration::ZERO);

    let waker = noop_waker();
    let mut cx = Context::from_waker(&waker);

    assert!(matches!(svc.poll_ready(&mut cx), Poll::Ready(Ok(()))));

    let panic = catch_unwind(AssertUnwindSafe(|| {
        let _ = svc.call("hello".to_string());
    }));

    assert!(panic.is_err());
    assert!(!called.load(Ordering::SeqCst));
}

This turns a configuration mistake into a runtime crash.

If period is supplied dynamically (config, env var, CLI flag, etc.), setting it to zero can bring down the process on the first request. Because poll_ready can succeed first, this is especially surprising for callers that assume a ready service can safely accept the next request.

Suggested fix

As a quick fix, reject Duration::ZERO at the API boundary (Hedge::new and Hedge::new_with_mock_latencies) and fail immediately with a clear message.