-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexandru Vasile <[email protected]>
Thanks for the PR. Please add a test that would have caught this. |
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
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] |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
This PR fixes an overflow produced by the
Instant::poll_tick
function when called for an instant set with a period fromu64::MAX
.For example, timers initiated like the following would panic because there's no room to add the 5 ms duration:
cc paritytech/polkadot-sdk#7793