Skip to content

Commit 9653034

Browse files
committed
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.
2 parents 443ae75 + 496f547 commit 9653034

File tree

2 files changed

+30
-6
lines changed

2 files changed

+30
-6
lines changed

src/libstd/sync/mpsc/mod.rs

+18-6
Original file line numberDiff line numberDiff line change
@@ -1311,12 +1311,13 @@ impl<T> Receiver<T> {
13111311
// Do an optimistic try_recv to avoid the performance impact of
13121312
// Instant::now() in the full-channel case.
13131313
match self.try_recv() {
1314-
Ok(result)
1315-
=> Ok(result),
1316-
Err(TryRecvError::Disconnected)
1317-
=> Err(RecvTimeoutError::Disconnected),
1318-
Err(TryRecvError::Empty)
1319-
=> self.recv_deadline(Instant::now() + timeout)
1314+
Ok(result) => Ok(result),
1315+
Err(TryRecvError::Disconnected) => Err(RecvTimeoutError::Disconnected),
1316+
Err(TryRecvError::Empty) => match Instant::now().checked_add(timeout) {
1317+
Some(deadline) => self.recv_deadline(deadline),
1318+
// So far in the future that it's practically the same as waiting indefinitely.
1319+
None => self.recv().map_err(RecvTimeoutError::from),
1320+
},
13201321
}
13211322
}
13221323

@@ -2301,6 +2302,17 @@ mod tests {
23012302
assert_eq!(recv_count, stress);
23022303
}
23032304

2305+
#[test]
2306+
fn very_long_recv_timeout_wont_panic() {
2307+
let (tx, rx) = channel::<()>();
2308+
let join_handle = thread::spawn(move || {
2309+
rx.recv_timeout(Duration::from_secs(u64::max_value()))
2310+
});
2311+
thread::sleep(Duration::from_secs(1));
2312+
assert!(tx.send(()).is_ok());
2313+
assert_eq!(join_handle.join().unwrap(), Ok(()));
2314+
}
2315+
23042316
#[test]
23052317
fn recv_a_lot() {
23062318
// Regression test that we don't run out of stack in scheduler context

src/libstd/time.rs

+12
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ impl Instant {
220220
impl Add<Duration> for Instant {
221221
type Output = Instant;
222222

223+
/// # Panics
224+
///
225+
/// This function may panic if the resulting point in time cannot be represented by the
226+
/// underlying data structure. See [`checked_add`] for a version without panic.
227+
///
228+
/// [`checked_add`]: ../../std/time/struct.Instant.html#method.checked_add
223229
fn add(self, other: Duration) -> Instant {
224230
self.checked_add(other)
225231
.expect("overflow when adding duration to instant")
@@ -387,6 +393,12 @@ impl SystemTime {
387393
impl Add<Duration> for SystemTime {
388394
type Output = SystemTime;
389395

396+
/// # Panics
397+
///
398+
/// This function may panic if the resulting point in time cannot be represented by the
399+
/// underlying data structure. See [`checked_add`] for a version without panic.
400+
///
401+
/// [`checked_add`]: ../../std/time/struct.SystemTime.html#method.checked_add
390402
fn add(self, dur: Duration) -> SystemTime {
391403
self.checked_add(dur)
392404
.expect("overflow when adding duration to instant")

0 commit comments

Comments
 (0)