Stream merge function calls poll_next again after returning Poll::Ready(None)

[phil-opp]

The Stream::poll_next should not be called again after it returned Ready(None), otherwise it "may panic, block forever, or cause other kinds of problems". The merge implementation does not seem to respect this.

For example, the following code will panic:

use futures_concurrency::stream::Merge;
use futures::StreamExt;

#[tokio::main(flavor = "current_thread")]
async fn main() {
    let create_stream = |count| futures::stream::unfold(count, |n| async move {
        if n > 1 {
            Some ((n, n - 1))
        } else {
            None
        }
    });
    let stream_1 = create_stream(10);
    let stream_2 = create_stream(10);
    
    let merged = (stream_1, stream_2).merge();
    println!("{:?}", merged.collect::<Vec<_>>().await);
}

The panic message is:

thread 'main' panicked at 'Unfold must not be polled after it returned Poll::Ready(None)'

Fusing the streams (i.e. create_stream(10).fuse()) fixes this, but this should not be needed, or enforced by the API.

[yoshuawuyts]

Oops, that's not good. I believe you already reported this a while back, so I apologize for the regression. Let's fix this ASAP.

[yoshuawuyts]

Okay, so I'm trying to repro it on the main branch, and the following test seems to pass without any problem:

    #[test]
    fn main() {
        block_on(async {
            let create_stream = |count| {
                futures::stream::unfold(count, |n| async move {
                    if n > 1 {
                        Some((n, n - 1))
                    } else {
                        None
                    }
                })
            };
            let stream_1 = create_stream(10);
            let stream_2 = create_stream(10);

            let merged = (stream_1, stream_2).merge();
            futures::StreamExt::collect::<Vec<_>>(merged).await;
            // merged.collect::<Vec<_>>().await;
        })
    }

I tried replacing future_lite::StreamExt with futures::StreamExt, and both seem to work. What version of futures-concurrency do you have in your Cargo.lock file?

[phil-opp]

Ah, I used the latest crates.io release, v7.0.0. I just retried with the latest master commit and it looks like the issue is fixed there.

[phil-opp]

I tried to pinpoint the commit and it looks like #96 fixed the issue.

[yoshuawuyts]

Okay! -- Seems like the right thing to do is then to publish a new release. On it!

[yoshuawuyts]

Published! https://github.com/yoshuawuyts/futures-concurrency/releases/tag/v7.1.0

[phil-opp]

Perfect, thanks a lot!