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

stream::Race never finishes #40

Open
connec opened this issue Feb 25, 2021 · 4 comments
Open

stream::Race never finishes #40

connec opened this issue Feb 25, 2021 · 4 comments

Comments

@connec
Copy link

connec commented Feb 25, 2021

I'm trying to use stream::Race to join a couple of timer-based streams that periodically emit events before terminating. Sadly, the current implementation of stream::Race will never terminate as it will always return Poll::Pending if no sub-stream polls Ready(Some(_)):

Poll::Pending

I'm happy to make a PR if this would be considered a bug, but I figured I'd check first since it could break users who rely on its non-termination.

@jbr
Copy link
Contributor

jbr commented Nov 22, 2021

I just ran into this again — it should either be documented clearly that Race will never end even if the component streams do (both/either), or this should be fixed

@taiki-e
Copy link
Collaborator

taiki-e commented Nov 23, 2021

I think the current implementation is wrong in 2 ways.

  • It should use fuse to avoid polling after the underlying stream returns None as it can cause panic.
  • If both underlying streams return None, then it should return None.

An example that implements this correctly is futures: https://github.com/rust-lang/futures-rs/blob/0.3.15/futures-util/src/stream/select.rs

So, I tend to fix the current behavior as a bug.

  • If you need the current behavior, you can use explicit .race(stream::pending()). (It also works with similar functions in other crates.)
  • Since the future is not polled after completion, the current implementation of future::race is fine.

@jbr
Copy link
Contributor

jbr commented Nov 23, 2021

The specific use case I had needed to terminate the stream when either of the streams terminated, not both. It seems like there are valid use cases and intuitive explanations for either "finish when both streams finish" and "finish when either stream finishes," depending on the type of content in the stream. Is there precedent for configuring this sort of thing on the Race stream? Or should there be a different name for those two different combinators?

@fogti
Copy link
Member

fogti commented Aug 23, 2023

so hm it might be a good idea to introduce FusedRace or such, which does have the described behavior (perhaps it could support both the "finish when both streams finish" and "finish when either stream finishes" via an enum argument)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants