Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time: Do not overflow to signal value #5710

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion tokio/src/runtime/time/entry.rs
Expand Up @@ -72,6 +72,10 @@ type TimerResult = Result<(), crate::time::error::Error>;
const STATE_DEREGISTERED: u64 = u64::MAX;
const STATE_PENDING_FIRE: u64 = STATE_DEREGISTERED - 1;
const STATE_MIN_VALUE: u64 = STATE_PENDING_FIRE;
/// The largest safe integer to use for ticks.
///
/// This value should be updated if any other signal values are added above.
pub(super) const MAX_SAFE_MILLIS_DURATION: u64 = u64::MAX - 2;

/// This structure holds the current shared state of the timer - its scheduled
/// time (if registered), or otherwise the result of the timer completing, as
Expand Down Expand Up @@ -126,7 +130,7 @@ impl StateCell {
fn when(&self) -> Option<u64> {
let cur_state = self.state.load(Ordering::Relaxed);

if cur_state == u64::MAX {
if cur_state == STATE_DEREGISTERED {
None
} else {
Some(cur_state)
Expand Down
2 changes: 1 addition & 1 deletion tokio/src/runtime/time/mod.rs
Expand Up @@ -8,7 +8,7 @@

mod entry;
pub(crate) use entry::TimerEntry;
use entry::{EntryList, TimerHandle, TimerShared};
use entry::{EntryList, TimerHandle, TimerShared, MAX_SAFE_MILLIS_DURATION};

mod handle;
pub(crate) use self::handle::Handle;
Expand Down
3 changes: 2 additions & 1 deletion tokio/src/runtime/time/source.rs
@@ -1,3 +1,4 @@
use super::MAX_SAFE_MILLIS_DURATION;
use crate::time::{Clock, Duration, Instant};

/// A structure which handles conversion from Instants to u64 timestamps.
Expand Down Expand Up @@ -25,7 +26,7 @@ impl TimeSource {
.unwrap_or_else(|| Duration::from_secs(0));
let ms = dur.as_millis();

ms.try_into().unwrap_or(u64::MAX)
ms.try_into().unwrap_or(MAX_SAFE_MILLIS_DURATION)
}

pub(crate) fn tick_to_duration(&self, t: u64) -> Duration {
Expand Down
13 changes: 13 additions & 0 deletions tokio/tests/time_sleep.rs
Expand Up @@ -267,6 +267,19 @@ async fn exactly_max() {
time::sleep(ms(MAX_DURATION)).await;
}

#[tokio::test]
async fn issue_5183() {
time::pause();

let big = std::time::Duration::from_secs(u64::MAX / 10);
// This is a workaround since awaiting sleep(big) will never finish.
tokio::select! {
biased;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
biased;
biased;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had it like that at first, but rustfmt disagree with it, so I have skipped the expression now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? Rustfmt usually does not format macros. Can you try and remove the skip annotation to see if our CI fails it?

Copy link
Contributor Author

@Erk- Erk- Jun 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did hence the force push above, here is the specific run of the CI
https://github.com/tokio-rs/tokio/actions/runs/5229606790/jobs/9442690852

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. Oh well.

_ = tokio::time::sleep(big) => {}
_ = tokio::time::sleep(std::time::Duration::from_nanos(1)) => {}
}
}

#[tokio::test]
async fn no_out_of_bounds_close_to_max() {
time::pause();
Expand Down