Decide how to handle stream Close then Reset

[Stebalien]

If a user calls Close() (close-write) and then Reset(), what should we do?

1. Ignore the reset. We probably shouldn't do this as we want to be able to tell the remote side to stop writing to us.
2. Send the reset but only apply it to the read side of the stream? This is probably the nicest thing to do.
3. If the stream is fully closed, do nothing. Otherwise, reset the entire stream (discarding any queued reads on the remote side).

Currently, we don't appear to do any of these consistently in go. IIRC, JS doesn't even expose Reset().

[vyzo]

I would say (2) seems best.

[Stebalien]

K. I've gone with (2) in libp2p/go-mplex#50.

[Stebalien]

I'm having trouble telling how js-libp2p handles this.

[marten-seemann]

QUIC does 1. Resetting only resets the write-side of a stream, so it would be nonsensical to close the read side of the stream in that case.

[Stebalien]

Resetting only resets the write-side of a stream, so it would be nonsensical to close the read side of the stream in that case.

As far as I can tell, it resets both (calls cancelread and cancelwrite).

[marten-seemann]

Then we have a pretty bad mismatch with QUIC. And resetting both sides makes a lot of application protocols impossible.

[Stebalien]

We do need to reconsider the interfaces but I'm not sure how this makes applications impossible.

Also, I'm talking about: https://github.com/libp2p/go-libp2p-quic-transport/blob/7424664a6de48f2cfd2e986b46ac997a6666ce06/stream.go#L14-L18.

[marten-seemann]

We do need to reconsider the interfaces but I'm not sure how this makes applications impossible.

Not in every application protocol is “I am terminating sending on this stream” equivalent to “I want you to terminate sending on this stream as well”.

Also, I'm talking about: https://github.com/libp2p/go-libp2p-quic-transport/blob/7424664a6de48f2cfd2e986b46ac997a6666ce06/stream.go#L14-L18.

Yes, this is ugly. The only reason we need this is because we have this mismatch in the stream interfaces. I’d love to get rid of this.

[raulk]

Let me summarise the semantics as I understand them, for passers-by, and for the sake of putting my thoughts in order:

1. Close() denotes we no longer have data to write on the stream. This triggers an io.EOF on the other end; they can Read() queued data until the point we sent the Close(), which is when the io.EOF will be raised. It is a graceful signal.
   • They can still continue sending data to us, until they Close().
2. Reset() -- this name confuses me every time. In my head, I map it to an ungraceful connection reset. However this is a signal we send programmatically to tell the other party we're gone, and they should stop writing to us, and that we have stopped reading. It obliterates any queued read data on the other end.
   • It's an abrupt closure; we're lacking a graceful abort (e.g. Finish())

Close() followed by Reset() means: I have stopped writing data to you, I want you to do the same.

Following this reasoning, option (3) is coherent with my interpretation of semantics, as well as with the declared behaviour: https://github.com/libp2p/go-stream-muxer/blob/211904436b52b258c78d701e52421532b6394107/muxer.go#L22-L23

cc @backkem @albrow

[vyzo]

I think we need something akin to reset that doesn't invalidate pending data written.
Perhaps we can call it Shutdown with semantics that close the output and send EOF (but don't invalidate pending data) and ignore all subsequent data sent by the peer.
The problem is that we have to use inet.FullClose which lingers a goroutine.

[marten-seemann]

@raulk

Close() followed by Reset() means: I have stopped writing data to you, I want you to do the same.

That doesn't cover the case where you stop sending data before you're done writing everything. Think for example when a client cancels a HTTP upload before you uploaded the whole file. Just sending the EOF is wrong, you need an explicit signal that you terminated sending. This however doesn't necessarily mean that you're not interested in the server's response, so the client's decision to stop sending on the stream shouldn't have any influence on the read-side of the stream.

Reset() -- this name confuses me every time.

You're not alone with that. When designing the API for quic-go, I decided to not have any Reset method for exactly that reason. Instead I opted for CancelWrite and CancelRead, which make it clear which side of the stream is affected by the operation. You can see the stream interface here: https://godoc.org/github.com/lucas-clemente/quic-go#Stream.

[raulk]

Think for example when a client cancels a HTTP upload before you uploaded the whole file. Just sending the EOF is wrong, you need an explicit signal that you terminated sending.

This doesn't need to be handled by the multiplexer IMO. In the case of HTTP 1.1, there are two ways you delimit the content being transferred, either by specifying the length through Content-Length, or in chunked mode by using CRLF and length-prefixed chunks. If the data stream is closed whether gracefully or ungracefully, the other party knows if it received the full blob or not.

In other words: Close() tells the other party: "expect no more content on this stream". Whether that's an expected or unexpected time to close a stream is up to the application protocol and FSM to determine.

[marten-seemann]

In the case of HTTP 1.1, there are two ways you delimit the content being transferred, either by specifying the length through Content-Length, or in chunked mode by using CRLF and length-prefixed chunks.

Content-Length can be -1 in HTTP, to indicate that the data will continue until the end of the connection / stream, so that doesn't works as a reliable signal that a transfer was canceled. When a client cancels an upload:

• in HTTP/1 it doesn't send a normal FIN, but RST on the TCP connection
• in HTTP/2, it doesn't send a DATA frame with the END_STREAM flag set, but it sends a RST_STREAM frame
• in HTTP/3, it doesn't send a STREAM frame with the FIN bit set, but a RESET_STREAM frame

In other words: Close() tells the other party: "expect no more content on this stream".

I'm not sure if that's compatible with how the io.Reader in Go is supposed to work. Reading an io.EOF from an io.Reader means that you've successfully read until the end of that reader. Otherwise, io.Reader is supposed to return an error different from io.EOF. Of course, you could argue that an application could wrap the stream and make sure that the right error code is returned, but why reimplement something at the application layer that belongs at the transport layer (see below for why it does).

Whether that's an expected or unexpected time to close a stream is up to the application protocol and FSM to determine.

Stream cancelations belong at the transport layer and not at the application layer is because normal and abnormal stream terminations come with different expectation about the reliable delivery of data. Both in TCP as well as in QUIC, after resetting the connection (or the stream, respectively), data sent prior to the reset doesn't need to be retransmitted if packets are lost. The only thing that needs to delivered reliably is the RST (or the RESET_STREAM frame, respectively). Depending on your network conditions, this can be a big performance difference.
Of course, due to the nature of HTTP/2 (and in general, stream multiplexers running on top of TCP), these kind of savings are not possible, since the underlying TCP connection forces data to be delivered reliably, simply because TCP is unaware of the streams running on top of it. This was one of the reasons people started working on a transport protocol on top of UDP. We should design our stream abstraction to take full advantage of that.