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

Update async.md #739

Merged
merged 2 commits into from
Dec 15, 2023
Merged

Update async.md #739

merged 2 commits into from
Dec 15, 2023

Conversation

ibeckermayer
Copy link
Contributor

Fixes an edge case bug in the Delay implementation wherein the waker could be awoken after having returned Poll::Ready if the Delay was short enough (e.g. 0).

Fixes an edge case bug in the Delay implementation wherein the waker could be awoken after having returned `Poll::Ready` if the Delay was short enough (e.g. `0`).
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Waking the waker after returning Poll::Ready is not actually incorrect. Spurious wakeups are always allowed.

Still, I don't mind this change.

@Darksonn
Copy link
Contributor

There's a compilation error:

---- target/debug/build/doc-test-8c33abc07f170500/out/doctests.rs - tokio::tutorial::async_md (line 685) stdout ----
error[E0053]: method `poll` has an incompatible type for trait
  --> target/debug/build/doc-test-8c33abc07f170500/out/doctests.rs:702:64
   |
19 |     fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<&'static str> {
   |                                                                ^^^^^^^^^^^^^^^^^^
   |                                                                |
   |                                                                expected `()`, found `&'static str`
   |                                                                help: change the output type to match the trait: `Poll<()>`
   |
   = note: expected signature `fn(Pin<&mut Delay>, &mut Context<'_>) -> Poll<()>`
              found signature `fn(Pin<&mut Delay>, &mut Context<'_>) -> Poll<&'static str>`

error[E0308]: mismatched types
  --> target/debug/build/doc-test-8c33abc07f170500/out/doctests.rs:706:32
   |
23 |             return Poll::Ready(());
   |                    ----------- ^^ expected `&str`, found `()`
   |                    |
   |                    arguments to this enum variant are incorrect
   |
help: the type constructed contains `()` due to the type of the argument passed
  --> target/debug/build/doc-test-8c33abc07f170500/out/doctests.rs:706:20
   |
23 |             return Poll::Ready(());
   |                    ^^^^^^^^^^^^--^
   |                                |
   |                                this argument influences the type of `Poll`
note: tuple variant defined here
  --> /rustc/a28077b28a02b92985b3a3faecf92813[155](https://github.com/tokio-rs/website/actions/runs/7178385919/job/19558862391?pr=739#step:5:156)f1ea1/library/core/src/task/poll.rs:17:5

error: aborting due to 2 previous errors

Fixes return type
@ibeckermayer
Copy link
Contributor Author

There's a compilation error:

Oops my bad, see the latest commit.

Waking the waker after returning Poll::Ready is not actually incorrect. Spurious wakeups are always allowed.

Interesting. In that case there may technically be a bug in the Mini Tokio implementation itself, at least the one I pieced together from the tutorial (which is admittedly substantively different from the full codebase the tutorial links to).

When I run it with Delay set to 0, I get

$ RUST_BACKTRACE=1 cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/mini_tokio`
Running mini_tokio...
delay 0 started!
delay 0 completed; out = "done"
thread 'main' panicked at src/main.rs:9:22:
`async fn` resumed after completion
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:117:5
   3: mini_tokio::main::{{closure}}
             at ./src/main.rs:9:22
   4: mini_tokio::Task::poll
             at ./src/lib.rs:125:17
   5: mini_tokio::MiniTokio::run
             at ./src/lib.rs:157:13
   6: mini_tokio::main
             at ./src/main.rs:21:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

(playground here, repo here)

@Darksonn
Copy link
Contributor

Ah, yes, that's very possible.

@Darksonn Darksonn merged commit 579e95d into tokio-rs:master Dec 15, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants