rust-lang / futures-rs

Zero-cost asynchronous programming in Rust
https://rust-lang.github.io/futures-rs/
Apache License 2.0
5.29k stars 607 forks source link

[Discussion] `Shared` seems to wake up the same waker that was polling it #2852

Open ZhennanWu opened 2 months ago

ZhennanWu commented 2 months ago

If I've not mistaken then Shared::poll will wake up the very same waker that was actually polling it, and return a Poll::Ready at the same time.

https://github.com/rust-lang/futures-rs/blob/bb63c376ca2d3f277f7bfb2b178c913faa00b947/futures-util/src/future/future/shared.rs#L347-L352

Which is confusing considering what std doc was saying about Waker::wake protocol.

  1. https://doc.rust-lang.org/std/future/trait.Future.html#tymethod.poll

    Once a task has been woken up, it should attempt to poll the future again

  2. https://doc.rust-lang.org/std/task/struct.Waker.html#method.wake

    it is guaranteed that each invocation of wake() (or wake_by_ref()) will be followed by at least one poll() of the task to which this Waker belongs

According to std doc, when implementing an executor, I should call poll at least once whenever wake was called. But this is not the case for Shared: it can return a Poll::Ready and call wake at the same time. If I follow a poll afterwards, it will return Poll::Ready again. It seems the most correct thing to do then is to not poll at all and ignore this wake, which contradicts with the doc.

This behavior itself may not be a problem, but the doc is quite confusing.

seanmonstar commented 2 months ago

Frankly, this line in the std docs appears wrong to me:

As long as the executor keeps running and the task is not finished, it is guaranteed that each invocation of wake() (or wake_by_ref()) will be followed by at least one poll() of the task to which this Waker belongs.

There's no way Waker can guarantee that. An executor probably would do that, but the strength of the sentence should reflect that level of certainty (read: not much).

Source: have implemented and maintained serious executors.