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

FusedFuture is incoherent #2779

Open
ijackson opened this issue Oct 11, 2023 · 0 comments
Open

FusedFuture is incoherent #2779

ijackson opened this issue Oct 11, 2023 · 0 comments

Comments

@ijackson
Copy link

The documentation for FusedFuture is quite vague. Unfortunately, I think this vagueness has enabled incoherence to arise and the situation is now rather poor:

Some reasoning

I think let r = select!{ r = fut => r }; should work the same way as let r = fut.await;. Otherwise select!'s behaviour is inconsistent with the behaviour of the supplied futures.

select! doesn't poll any futures whose FusedFuture::is_terminated returns true.

This means that FusedFuture::is_terminated must not return true if the future would return Ready; nor if it would squirrel away a Waker from the Context.

select!'s complete => feature relies on is_terminated. The documentation is not entirely clear, but certainly is_terminated there must return true if the future has already returned Ready.

The most reasonable interpretation of all this is_terminated should return precisely whether the future has already returned Ready.

However, many futures in futures-rs implement FusedFuture despite being unable to answer this question definitively. For example, OptionFuture, oneshot::Receiver and probably others. Others answer it wrongly: for example, future::Pending has is_terminated always return true and this is even used in a test case (async_await_macros.rs, select_with_complete_can_be_used_as_expression).

Options

  1. Declare FusedFuture::is_terminated is supposed to say precisely whether the future has returned Ready. Remove the impls for futures that don't know whether they've already returned Ready (which is many of them) and have users who need this functionality call .fuse(). Fix future::Pending. etc. This is an API break.

  2. Leave FusedFuture::is_terminated as it is, but plaster it, and complete =>, with big warnings saying it's deprecated. Have select! without complete not ever call is_terminated. This fixes the behaviour of select! to be cosistent with await, and leaves users of complete alone. This is arguably a bugfix, although it might need a crater run or something. It seems like a practical approach to making this better, pending the probably still-long-distant 1.0 release.

  3. As 2, then: Introduce a new version of complete with a new name and a new trait to support it. For use in tests and corner cases, introduce a new "always looks like it has been awaited" future which is always Pending and (unlike future::Pending) is always "complete" for the purposes of select!.

  4. Tell everyone to use Tokio which doesn't have these bugs.

References

#1989 request for select! to work without FusedFuture, based on conditional branches (which Tokio's select! has). Fairly extensive discussion of some other problems with FusedFuture.

#2475 report of brokenness with oneshot::Receiver

#2455 report of confusion with OptionFuture and Pending

#2213 trouble with future::Map

FusedStream

This trait is very similar to FusedFuture. My examination of the source in futures-rs suggests that at least some of its impls have similar bugs to the bugs in the impls of FusedFuture. But I haven't investigated FusedStream properly and don't really know why it exists (and why it has a different API approach to FusedIterator)

darsto added a commit to darsto/borrow_mutex that referenced this issue Apr 21, 2024
This isn't used. We could opt for implementing FusedFuture and
FusedStream, but these traits seem incoherent [1] and are due
to removal for futures-core 1.0 [2]. They're not needed, so just
get rid of them.

[1] rust-lang/futures-rs#2779
[2] rust-lang/futures-rs#2207
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

No branches or pull requests

1 participant