-
Notifications
You must be signed in to change notification settings - Fork 633
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
Fix incorrect termination of select_with_strategy
streams
#2635
Fix incorrect termination of select_with_strategy
streams
#2635
Conversation
So, the Note the lines above your change:
Which reads Admittedly I should probably have used a differently named enum that's isomorphic to @Sushisource What kind of funky behaviour are you seeing exactly? The tests are in |
@414owen I've added a failing test. The problem behavior is that the left stream never terminated, but once the right stream does, if my strategy is to always poll left, the
Think I've figured it out and fixed it. Problem was the |
8088366
to
3b8ad5c
Compare
3b8ad5c
to
74e2c4b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sushisource Nice work. Great to have a test case covering this. 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Sushisource for the fix, and thanks @414owen for the review!
👋 we've faced the same issue, nice to see that has been fixed 👍 |
Published in 0.3.24. |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [futures](https://rust-lang.github.io/futures-rs) ([source](https://github.com/rust-lang/futures-rs)) | dependencies | patch | `0.3.23` -> `0.3.24` | --- ### Release Notes <details> <summary>rust-lang/futures-rs</summary> ### [`v0.3.24`](https://github.com/rust-lang/futures-rs/blob/HEAD/CHANGELOG.md#​0324---2022-08-29) [Compare Source](rust-lang/futures-rs@0.3.23...0.3.24) - Fix incorrect termination of `select_with_strategy` streams ([#​2635](rust-lang/futures-rs#2635)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xODEuMCIsInVwZGF0ZWRJblZlciI6IjMyLjE4MS4wIn0=--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1529 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Fixes #2634
I can add a test tomorrow (not sure exactly where, if I get a pointer that would be lovely) or maintainers are welcome to add it to my PR.