search

libp2p / go-libp2p-circuit

Circuit Switching for libp2p
MIT License
48 stars 16 forks source link

It should be possible to close a stream and then read from it. #18

Open Stebalien opened 7 years ago

Stebalien commented 7 years ago

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.

vyzo commented 7 years ago

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.

Stebalien commented 7 years ago

IMO, the close should come after the last write. That is, close means "I'm done sending requests".

Ichbinjoe commented 5 years ago

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.

Stebalien commented 5 years ago

@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.

Ichbinjoe commented 5 years ago

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.

jsign commented 4 years ago

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 go-libp2p@v0.4.2 (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?