Fix incorrect termination of select_with_strategy streams

[Sushisource]

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.

[414owen]

So, the finish function uses ps: PollNext to mean the stream that you want to mark as finished.
When the internal state is LeftFinished and you're marking Right as finished, the internal state should transition to BothFinished.

Note the lines above your change:

(InternalState::Start, PollNext::Right) => {
  *self = InternalState::RightFinished;
}

Which reads when neither are finished and we mark Right as finished, then Right is Finished.

Admittedly I should probably have used a differently named enum that's isomorphic to PollNext for ps.

@Sushisource What kind of funky behaviour are you seeing exactly? The tests are in futures/tests/stream.rs, so a good first step would be to add a failing test there :)

[Sushisource]

@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 select_with_strategy stream then terminates.

And, indeed, what I changed hasn't fixed it. I'll poke around a bit and see what would, or if you see this first and know what the problem is, awesome :)

Think I've figured it out and fixed it. Problem was the poll_inner check was returning Poll::Ready(None) when the "other" stream is done, even if the first stream was not yet done.

[414owen]

@Sushisource Nice work. Great to have a test case covering this. 🚀

[taiki-e]

Thanks @Sushisource for the fix, and thanks @414owen for the review!

[QuentinPerez]

👋 we've faced the same issue, nice to see that has been fixed 👍
Do you think that we could have a new minor with that patch soon ?

[taiki-e]

Published in 0.3.24.