search

hashicorp / yamux

Golang connection multiplexing library
Mozilla Public License 2.0
2.19k stars 232 forks source link

Data lost when closing Session #86

Closed yipal closed 2 years ago

yipal commented 3 years ago

Hi all, I ran into an issue, and I wanted to get your opinion on what the correct behavior should be.

I have a yamux client that writes data to a Stream, and then closes the Stream, and also the Session. The problem is that some times, all the data arrives at the other end, and other times, some data remains in the send queue when the underlying connection actually closes.

First, this causes this warning message: [ERR] yamux: Failed to write header: tls: use of closed connection

Second, the data does not arrive at the other end.

This commit contains a unit test that shows what happens; the failure happens only sometimes because it is a race condition: https://github.com/yipal/yamux/commit/a5eed1ecdd111ad44f819b5f4f831c5065ab542d

I noticed that the existing code finishes sending the data when the client calls Stream.Close(), so that makes me think the correct behavior is for Session.Close() to also finish draining the send queue before actually closing the underlying connection.

r0l1 commented 3 years ago

I tested it with our yamux fork and I can't reproduce the error. Could you please recheck? https://github.com/desertbit/yamux

Edit: we made some changes to the closing functionality of yamux to be compatible for our orbit rpc project. Would be good to know, if this bug effects also our fork.

evanphx commented 2 years ago

The test in https://github.com/yipal/yamux/commit/a5eed1ecdd111ad44f819b5f4f831c5065ab542d currently passes fine, so I'm assuming the bug is gone.