-
Notifications
You must be signed in to change notification settings - Fork 75
Question: Why doesn't Close() close the stream? #9
Comments
So, This is clearly not a very nice API and was a terrible mistake. I planned to address this once we merged the go-libp2p-core refractor and now we have. |
This changes the behavior of `Close` to behave as one would expect: it closes the stream. The new methods, CloseWrite/CloseRead allow for closing the stream in a single direction. Note: This _does not_ implement CancelWrite/CancelRead as our stream muxer _protocols_ don't support that. fixes #9
I don't think this is a mistake. By the way, this exactly how things work for QUIC streams as well. And for TCP connections, if I remember correctly. |
The issue is at the API layer, not the protocol layer. That is, users expect We need to support closing for reading/writing, but we should do the thing users expect by default. |
This changes the behavior of `Close` to behave as one would expect: it closes the stream. The new methods, CloseWrite/CloseRead allow for closing the stream in a single direction. Note: This _does not_ implement CancelWrite/CancelRead as our stream muxer _protocols_ don't support that. fixes #9
* add CloseRead/CloseWrite on streams This changes the behavior of `Close` to behave as one would expect: it closes the stream. The new methods, CloseWrite/CloseRead allow for closing the stream in a single direction. Note: This _does not_ implement CancelWrite/CancelRead as our stream muxer _protocols_ don't support that. fixes #9 * remove stream util helpers FullClose and AwaitEOF were introduced to work around the fact that calling Close on a stream only closed the write half. All users must adapt their code to the new interfaces, so this change is intentionally breaking.
I don't understand this bit of the API. When we are handling Closer interfaces, we close them when we are done using the resource. However, this Closer doesn't actually close the stream. It seems we have to call
Reset
which actually sends an error to the peer.go-libp2p-core/mux/mux.go
Lines 22 to 24 in bac72eb
We have had some pain with this when trying the following logic:
For the second step, if we call
Reset
then the peer will receive the error only and does not process the message. However, if we callClose
and the peer doesn't then we have this dangling connection that we don't care about.The text was updated successfully, but these errors were encountered: