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

tokio/interval: Fix overflow for interval tick #7193

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lexnv
Copy link

@lexnv lexnv commented Mar 4, 2025

This PR fixes an overflow produced by the Instant::poll_tick function when called for an instant set with a period from u64::MAX.

For example, timers initiated like the following would panic because there's no room to add the 5 ms duration:

let mut timer = tokio::time::interval(std::time::Duration::from_secs(u64::MAX));

cc paritytech/polkadot-sdk#7793

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-time Module: tokio/time labels Mar 4, 2025
@Darksonn
Copy link
Contributor

Darksonn commented Mar 4, 2025

Thanks for the PR. Please add a test that would have caught this.

@lexnv
Copy link
Author

lexnv commented Mar 4, 2025

Thanks for the swift review! 🙏

I've added a test and patched the missed tick behavior as well

@@ -470,8 +470,25 @@ async fn stream_with_interval_poll_tick_no_waking() {
assert_eq!(items, vec![]);
}

#[tokio::test(start_paused = true)]
#[tokio::test]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't this test use start_paused?

Copy link
Author

Choose a reason for hiding this comment

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

We need a bit of delay between two subsequent poll_tick calls (5ms) to enter the misstick behavior and trigger the overflow panic

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, a test that takes 2 seconds to complete is not acceptable. I'm quite certain that the test works just fine with start_paused - the parameter should not change the behavior of tests that rely on time other than making them run faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-time Module: tokio/time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants