nervosnetwork / tentacle

A multiplexed p2p network framework that supports custom protocols
https://docs.rs/tentacle
MIT License
54 stars 24 forks source link

The stream is not closed when the underlying connection closed? #259

Closed abel-von closed 4 years ago

abel-von commented 4 years ago

Hi, I'm using tokio-yamux in my project, and this tokio-yamux helps me a lot, thanks for this excellent lib.

I have encountered a problem.

I opened some streams, and copy the io from my served connections to those stream and also from those streams to my served connections. I have tested an exceptional use case that when I close the server side of yamux, I expect that the copying threads should stop. but the reality is that it is still running.

So my question is: How to stop the Read or Write of the yamux stream, when the underlying connection is closed. I tried to add control.close() in the "next call thread" but it is still no use.

tokio::spawn(async move {
                loop {
                    match agent_session.next().await {
                        Some(_) => (),
                        None => {
                            debug!("closing session");
                            health_control.close().await;
                            break;
                        }
                    }
                }
            });

Is it something I ignored? or are there any other suggestion for this use case?

abel-von commented 4 years ago

the copying thread is like below:

                   let (mut rback, mut wback) = serve_stream.split();
                   let (mut rfront, mut wfront) = tokio::io::split(agent_stream);
                   let client_to_server = async {
                        let r = tokio::io::copy(&mut rfront, &mut wback)
                            .await
                            .map_err(io_error!(e, "failed to copy from client to server"));
                        wback
                            .shutdown()
                            .await
                            .map_err(io_error!(e, "failed to shutdown proxy socket"))?;
                        r
                    };
                    let server_to_client = async {
                        let r = tokio::io::copy(&mut rback, &mut wfront)
                            .await
                            .map_err(io_error!(e, "failed to copy from server to client"));
                        wfront
                            .shutdown()
                            .await
                            .map_err(io_error!(e, "failed to shutdown agent socket"))?;
                        r
                    };

                    futures::try_join!(client_to_server, server_to_client)
                        .map_err(|e| {
                            let msg = format!("failed to copy stream: {}", e);
                            error!("{}", msg.as_str());
                            Other(msg)
                        })
driftluo commented 4 years ago

After my review, this is a missing state machine transition, we didn't have this scenario before so didn't find it, thanks!

I'll fix it and release it as soon as I can, but if you're in a hurry, you can use the commit that PR fixes afterward first.

driftluo commented 4 years ago

@abel-von Ok, now, you can try #260 pr