Auto merge of #56827 - faern:eliminate-recv-timeout-panic, r=KodrAus
Eliminate Receiver::recv_timeout panic Fixes #54552. This panic is because `recv_timeout` uses `Instant::now() + timeout` internally. This possible panic is not mentioned in the documentation for this method. Very recently we merged (still unstable) support for checked addition (#56490) of `Instant + Duration`, so it's now finally possible to add these together without risking a panic.
This commit is contained in:
commit
96530344ef
2 changed files with 30 additions and 6 deletions
|
@ -1311,12 +1311,13 @@ impl<T> Receiver<T> {
|
|||
// Do an optimistic try_recv to avoid the performance impact of
|
||||
// Instant::now() in the full-channel case.
|
||||
match self.try_recv() {
|
||||
Ok(result)
|
||||
=> Ok(result),
|
||||
Err(TryRecvError::Disconnected)
|
||||
=> Err(RecvTimeoutError::Disconnected),
|
||||
Err(TryRecvError::Empty)
|
||||
=> self.recv_deadline(Instant::now() + timeout)
|
||||
Ok(result) => Ok(result),
|
||||
Err(TryRecvError::Disconnected) => Err(RecvTimeoutError::Disconnected),
|
||||
Err(TryRecvError::Empty) => match Instant::now().checked_add(timeout) {
|
||||
Some(deadline) => self.recv_deadline(deadline),
|
||||
// So far in the future that it's practically the same as waiting indefinitely.
|
||||
None => self.recv().map_err(RecvTimeoutError::from),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -2301,6 +2302,17 @@ mod tests {
|
|||
assert_eq!(recv_count, stress);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn very_long_recv_timeout_wont_panic() {
|
||||
let (tx, rx) = channel::<()>();
|
||||
let join_handle = thread::spawn(move || {
|
||||
rx.recv_timeout(Duration::from_secs(u64::max_value()))
|
||||
});
|
||||
thread::sleep(Duration::from_secs(1));
|
||||
assert!(tx.send(()).is_ok());
|
||||
assert_eq!(join_handle.join().unwrap(), Ok(()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recv_a_lot() {
|
||||
// Regression test that we don't run out of stack in scheduler context
|
||||
|
|
|
@ -220,6 +220,12 @@ impl Instant {
|
|||
impl Add<Duration> for Instant {
|
||||
type Output = Instant;
|
||||
|
||||
/// # Panics
|
||||
///
|
||||
/// This function may panic if the resulting point in time cannot be represented by the
|
||||
/// underlying data structure. See [`checked_add`] for a version without panic.
|
||||
///
|
||||
/// [`checked_add`]: ../../std/time/struct.Instant.html#method.checked_add
|
||||
fn add(self, other: Duration) -> Instant {
|
||||
self.checked_add(other)
|
||||
.expect("overflow when adding duration to instant")
|
||||
|
@ -387,6 +393,12 @@ impl SystemTime {
|
|||
impl Add<Duration> for SystemTime {
|
||||
type Output = SystemTime;
|
||||
|
||||
/// # Panics
|
||||
///
|
||||
/// This function may panic if the resulting point in time cannot be represented by the
|
||||
/// underlying data structure. See [`checked_add`] for a version without panic.
|
||||
///
|
||||
/// [`checked_add`]: ../../std/time/struct.SystemTime.html#method.checked_add
|
||||
fn add(self, dur: Duration) -> SystemTime {
|
||||
self.checked_add(dur)
|
||||
.expect("overflow when adding duration to instant")
|
||||
|
|
Loading…
Add table
Reference in a new issue