go-libp2p-circuit
go-libp2p-circuit copied to clipboard
libp2p
It should be possible to close a stream and then read from it.
Putting this close call before the read causes the tests to fail. However, given that we're now allow half-closed streams, it should work.
It looks like something is seeing that the stream is closed and resetting the connection before responding.
So is this just a subtlety in the code? Semantically, the close should be after the read, but perhaps we should add a comment there.
IMO, the close should come after the last write. That is, close means "I'm done sending requests".
This no longer seems to be the case as of now.
https://github.com/Ichbinjoe/go-libp2p-circuit/blob/preemptive-close/relay.go#L217
I made these changes and the tests appear to still pass. Whatever issue exists no longer seems to now.
@Ichbinjoe mind making a PR? We'll need to make sure it doesn't break js-libp2p but it would be nice to get this fix in.
I currently don't have the JS stack setup, but I'm sure over the next couple of days I can mess around with it more.
While doing a toy-app for an article, I wanted to be somewhat strict with the documentation. If I navigate to the documented semantics of close, we can find this.
// Close closes the stream for writing. Reading will still work (that is, the remote side can still write).
In the toy-app case, the side that opened the stream would only read stuff, so guided by that comment, I switched a defered Close(), to an immediate Close() (before reading the stream), which lead to a stream reset error.
Am I talking about the same thing discussed here? I'm running just [email protected] (asking since this isn't that repo). If that's the case, maybe it would be better to delete that comment since it may mislead to a usage that doesn't behave as expected?
