futures-rs icon indicating copy to clipboard operation
futures-rs copied to clipboard

OptionFuture is_terminated design confusion

Open ComputerDruid opened this issue 4 years ago • 2 comments

OptionFuture's design seems inconsistent to me.

The docs don't clearly explain what it's for:

A future representing a value which may or may not be present.

Which could describe any Future<Output = Option<_>>.

Given that it's constructed from an Option<Future> though, it seems clear that it's an adapter for Option<Future> that can be awaited, yielding Some(output) when the future is present, and None when it's absent.

However, it also implements FusedFuture (where F: FusedFuture at least) so that OptionFuture::from(None).is_terminated() == true which seems inconsistent with that interpretation. is_terminated is documented as meaning:

Returns true if the underlying future should no longer be polled.

Which sort of means you're not supposed to await it if it's none?

And practically speaking, FusedFuture affects the behavior of select!, which will skip over futures that are terminated (so it can safely be used in a loop). OptionFuture's current implementation will result in it getting skipped entirely for the None case which is surprising if you're just expecting it to be an adapter that'll produce None.

One might suggest that this None-skipping behavior is desirable, as a solution for #2270. But for that usecase, why is the result wrapped in a Some()? And why can you create an OptionFuture from a !FusedFuture which doesn't work this way?

A third option (pun not intended) is suggested by #2457 which is to have none mean terminated and have the inner transition to None after the future completes. Which would make OptionFuture::from(None) very similar to Fuse::terminated(), except polling/awaiting it yields None rather than panicking.

Personally I think the adapter interpretation is correct, and OptionFuture should not implement FusedFuture. If that's desirable I'd be happy to send a PR. But I admit I haven't personally found a use for OptionFuture in code I've written, so maybe I'm missing the point here?

ComputerDruid avatar Aug 17 '21 20:08 ComputerDruid

This question is also in Pending, which impl FusedFuture with always true.

select! will ignore it by Fuse::terminated() == true.

https://github.com/rust-lang/futures-rs/blob/c0e936802828c37f8f961f91ed2c70c513878042/futures-macro/src/select.rs#L206

https://github.com/rust-lang/futures-rs/blob/c0e936802828c37f8f961f91ed2c70c513878042/futures-macro/src/select.rs#L314

https://github.com/rust-lang/futures-rs/blob/c0e936802828c37f8f961f91ed2c70c513878042/futures-macro/src/select.rs#L319

This problem can cause strange behavior:

let mut a_fut = future::pending::<()>();
let mut b_fut = future::pending::<()>();

loop {
    select! {
        _ = a_fut => (),
        _ = b_fut => (),
        complete => break,
        default => panic!(), // never runs (futures run first, then complete)
    };
}
println!("1") // will print out
future::pending::<()>().await;
println!("1") // never print out

driftluo avatar Sep 03 '21 11:09 driftluo

This question is also in Pending, which impl FusedFuture with always true.

select! will ignore it by Fuse::terminated() == true.

I find this behaviour to be very confusing and unexpected.

I fully expected the behaviour of select! with pending to be equivalent between these futures, but it is not:

let a_mut = future::pending::<()>();
let b_mut = async {
    future::pending::<()>().await;
    ()
}
.boxed()
.fuse();

extremeandy avatar Jul 14 '22 06:07 extremeandy