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

Wrong error message if timer deadline is greater than two years #1953

Closed
benesch opened this issue Dec 13, 2019 · 0 comments
Closed

Wrong error message if timer deadline is greater than two years #1953

benesch opened this issue Dec 13, 2019 · 0 comments
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-time Module: tokio/time

Comments

@benesch
Copy link
Contributor

benesch commented Dec 13, 2019

Tokio appears to have an undocumented limit of two years for timer deadlines. At least, I wasn't able to find any mention of this in the documentation, but saw this in the source code:

/// Number of levels. Each level has 64 slots. By using 6 levels with 64 slots
/// each, the timer is able to track time up to 2 years into the future with a
/// precision of 1 millisecond.
const NUM_LEVELS: usize = 6;

Things go a bit haywire if you create a future with a timeout more than two years in the future. Here's a Tokio test that demonstrates the issue:

diff --git a/tokio/tests/time_timeout.rs b/tokio/tests/time_timeout.rs
index 4efcd8ca..f604f024 100644
--- a/tokio/tests/time_timeout.rs
+++ b/tokio/tests/time_timeout.rs
@@ -74,6 +74,32 @@ async fn future_and_timeout_in_future() {
     assert_ready_ok!(fut.poll()).unwrap();
 }
 
+#[tokio::test]
+async fn future_and_huge_timeout_in_future() {
+    const YR_5: u64 = 5 * 365 * 24 * 60 * 60 * 1000;
+
+    time::pause();
+
+    // Not yet complete
+    let (tx, rx) = oneshot::channel();
+
+    // Wrap it with a deadline
+    let mut fut = task::spawn(timeout(ms(YR_5), rx));
+
+    // Ready!
+    assert_pending!(fut.poll());
+
+    // Turn the timer, it runs for the elapsed time
+    time::advance(ms(90)).await;
+
+    assert_pending!(fut.poll());
+
+    // Complete the future
+    tx.send(()).unwrap();
+
+    assert_ready_ok!(fut.poll()).unwrap();
+}
+
 #[tokio::test]
 async fn deadline_now_elapses() {
     use futures::future::pending;

Here's the output of that test:

$ cargo test --features=full --test time_timeout -- future_and_huge_timeout_in_future
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running target/debug/deps/time_timeout-595f8b43357499a9

running 1 test
test future_and_huge_timeout_in_future ... FAILED

failures:

---- future_and_huge_timeout_in_future stdout ----
thread 'future_and_huge_timeout_in_future' panicked at 'timer error: timer is shutdown', tokio/src/time/delay.rs:96:23
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.


failures:
    future_and_huge_timeout_in_future

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 6 filtered out

The error message that the timer instance is shutdown is pretty confusing—I spent an hour or two thinking that my runtime wasn't starting up the timer instance properly. (It was.) The problem is that the information about why a timer entry has errors is thrown away, and always assumed to be because the timer shutdown. For example, here's where the error message from the wheel about the invalid duration is thrown away:

entry.error();

I'm also pretty sure it's impossible to ever see time::Error::AtCapacity, since that information appears to be thrown away here:

if inner.increment().is_err() {
entry = Entry::new2(deadline, duration, Weak::new(), ERROR)

These errors are all converted into shutdown errors here:

Err(Error::shutdown())

I'm happy to try to fix this with some guidance, but there are definitely some questions to answer first! Should there be an additional time::Error variant named InvalidDuration or something? And how should the error type be threaded through the entry? Via additional magic numbers, like u64::MAX - 1, in the state field, perhaps?

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 C-bug Category: This is a bug. M-time Module: tokio/time
Projects
None yet
Development

No branches or pull requests

2 participants