test against the libp2p transport tests

[Stebalien]

This will fail until #9 is fixed.

[Stebalien]

Unfortunately, I couldn't use ginkgo as I needed a testing.T.

[marten-seemann]

@Stebalien: I just fixed #9. Can you please rebase this, I'd like to see the CI build pass before merging this?

[Stebalien]

So, it looks like if I close a stream and then close the connection, reading from the stream on the other side will return a PeerGoingAway error instead of an EOF. This means that the following won't work:

1. accept a connection
2. accept a stream
3. read a request from the stream (followed by an EOF)
4. send a response on the stream
5. close the stream (sending an EOF)
6. close the connection.

[marten-seemann]

The error you'll get depends on the order things are processed. If the stream is read from before the connection is closed, it will return EOF. If afterwards, it returns the close error.

In general, the protocol you describe is inherently racy when run on top of QUIC, since UDP packets can be reordered on the wire. The packet containing the CONNECTION_CLOSE frame can arrive before the packets containing all the stream data are received. The stream will then return the error carried in that frame.
Furthermore, allowing Reads from streams after the connection was closed is not an option either, since closing of the connection means that no data will be retransmitted, so the EOF might never arrive if the packet carrying it gets lost.

The way to fix this by having the peer that is reading the data close the connection once it finished reading.

[Stebalien]

So, this is a QUIC issue, not a UDP issue. QUIC could solve this by having close simply mean that no new streams will be opened. When sending a close, an implementation would immediately reset any open streams. When receiving a close, the receiver would also reset any open streams, sending back a close message. Once the last stream has been closed/reset, the connection would be fully closed.

To immediately kill the connection, the user would reset it.

This can be implemented as a protocol on-top-of QUIC by opening a stream and telling the other side to close the connection once they've all of the streams have been closed. On the other hand, not having a way to gracefully shut-down a connection ASAP in the protocol itself feels like a massive oversight.

---

However, I see that this is simply not how one is supposed to handle QUIC connections. With QUIC, one isn't really supposed to close a connection. Instead, one simply stops using it and lets it go idle.

Luckily, this is how libp2p uses connections as well. However, it obviously breaks all of our transport tests... I'll have to fix those.

[marten-seemann]

So, this is a QUIC issue, not a UDP issue. QUIC could solve this by having close simply mean that no new streams will be opened. When sending a close, an implementation would immediately reset any open streams. When receiving a close, the receiver would also reset any open streams, sending back a close message. Once the last stream has been closed/reset, the connection would be fully closed.

Right, that's one way you could do it. Closing of connections was discussed in the QUIC working group, and we came to the conclusion that an orderly shutdown belongs in the application layer, not the transport. There are a lot of ways applications shut down, so the transport itself can only provide the "immediate close" function.
For example, in the classic HTTP/QUIC server <-> browser case, closing is easy: As soon as the client has all the data it wants, it just closes the connection. Bidirectional protocols are more complicated, since both peers need to signal each other that they're done. By the way, this even applies if you're using a stream muxer on top of TCP.

However, I see that this is simply not how one is supposed to handle QUIC connections. With QUIC, one isn't really supposed to close a connection. Instead, one simply stops using it and lets it go idle.

Luckily, this is how libp2p uses connections as well.

That should work for now.

[Stebalien]

@marten-seemann could you look into the test failure? quic-go/quic-go#2097.

[Stebalien]

This has been rebased and updated. It should be ready to merge.

[marten-seemann]

Looks like the tests are stalling, Travis is reporting no output for 10 minutes.

[Stebalien]

Damn. It passed locally.

[Stebalien]

Unfortunately, I don't have time to keep working on this and it's going to sit here till someone picks it up. @marten-seemann, could you see this through? The transport tests are pretty exhaustive so it would be nice to test quic against them.

[marten-seemann]

@Stebalien Ok, I'll take it over from here.

[Stebalien]

Thanks!

[marten-seemann]

-> #93.