Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connections: Document stream behaviour #469

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion connections/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ and spec status.
- [multistream-select](#multistream-select)
- [Upgrading Connections](#upgrading-connections)
- [Opening New Streams Over a Connection](#opening-new-streams-over-a-connection)
- [Stream Behaviour](#stream-behaviour)
- [States](#states)
- [Resets](#resets)
- [State transition diagram](#state-transition-diagram)
- [Practical Considerations](#practical-considerations)
- [Interoperability](#interoperability)
- [State Management](#state-management)
Expand Down Expand Up @@ -72,7 +76,7 @@ verifiable by the other peer.
libp2p connection. They must support backpressure, which prevents receivers from
being flooded by data from eager senders. They can also be "half closed",
meaning that a stream can be closed for writing data but still open to receiving
data and vice versa.
data and vice versa. See [Stream Behaviour](#stream-behaviour) for details.

Support for multiple streams ensures that a single connection between peers can
support a wide variety of interactions, each with their own protocol. This is
Expand Down Expand Up @@ -263,6 +267,49 @@ indicating whether the handler supports the protocol. This allows more flexible
behavior than exact literal matching, which is the default behavior if no match
function is provided.

## Stream Behaviour

This section specifies the expected behaviour of a libp2p stream.

### States

A libp2p stream can be in one of four states:

- Open for reading and writing
- Open for reading only
- Open for writing only
- Closed

The default state after opening a new stream is "Open for reading and writing".
Either party can at any point close the reading or writing side.

The "Closed" state is the product of both halfs (read and write) of the stream being closed.
This can happen because one party closes both halfs but also as a combination
of both parties closing their write-half (which automatically closes other's read-half).

Writing to a write-closed stream SHOULD fail with an error clearly signalling the closed state.

Reading from a read-closed stream SHOULD succeed by reading 0 bytes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intuitively this is to implementation specific. E.g. some may express this through a special event, some might express this through 0 bytes. I don't think we should dictate the way of how an implementation expresses this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Reading from a read-closed stream SHOULD succeed by reading 0 bytes.
Reading from a read-closed stream SHOULD succeed.

Would that be better?

This allows for a robust "read to end"-behaviour.
Layers on top of the stream MAY fail with an unexpected EOF error in case more data was expected.
Comment on lines +293 to +294
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needs to be in the specification.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you just omit it? I think I'd find it helpful because it explains, why we want read operations on closed streams to be successful. Without this, people may rightfully ask, why reading from a read-closed stream shouldn't fail?

I am happy to reword it, but I think it is useful to have in the spec in one form or another.


### Resets

A stream reset is an abrupt termination of the stream.
Implementations SHOULD reset a stream to signal a non-graceful close.
Typically, this is during a resource cleanup like garbage-collection for streams
which have not been closed properly (see [States](#states) above).

Upon receiving a stream reset, any buffered data in the stream MAY be discarded.

Reading and writing to a reset stream SHOULD fail with an error clearly signalling the reset.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross referencing a past discussion for the curious reader here, namely that QUIC only closes the write side.

#412 (comment)

I don't have a strong opinion here. Maybe @marten-seemann does. Ideally we would be consistent across the different multiplexers. At least we should document the difference here.


### State transition diagram

The following diagram illustrates the state transitions:

![https://www.planttext.com/?text=VP8nRy8m48Nt-nKd690gUUZAK14wT6ibEZ04XXGV4CdOw7mHwh_d70XDIDB5ikzzx-xPyY9AmLAT782KuWY_XQauePQ58ku3urc1Nym0yfSj6lE6NsVo06b5m-NXAFdqnrMqLMdDfT2x2v7i79UuIxk8brJj6WvCv7kEh75e1ditEDgt1gnKwFNlqO_kRJmRYfDFcMmY6sf5aGJWppZAT94cu72uBlk8Dn8DMe_oFrytVw97azoQFz73rmSVX71Yz6onOD91MeyRR_0ZXQbheM8C5ztlf0o-5fSwkzRagBekz-ypYmqrmIBYvol0WhpLVtS5](https://www.planttext.com/api/plantuml/svg/VP8nRy8m48Nt-nKd690gUUZAK14wT6ibEZ04XXGV4CdOw7mHwh_d70XDIDB5ikzzx-xPyY9AmLAT782KuWY_XQauePQ58ku3urc1Nym0yfSj6lE6NsVo06b5m-NXAFdqnrMqLMdDfT2x2v7i79UuIxk8brJj6WvCv7kEh75e1ditEDgt1gnKwFNlqO_kRJmRYfDFcMmY6sf5aGJWppZAT94cu72uBlk8Dn8DMe_oFrytVw97azoQFz73rmSVX71Yz6onOD91MeyRR_0ZXQbheM8C5ztlf0o-5fSwkzRagBekz-ypYmqrmIBYvol0WhpLVtS5)

## Practical Considerations

This section will go over a few aspects of connection establishment and state
Expand Down
5 changes: 2 additions & 3 deletions mplex/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,8 @@ To close a stream, send a message with a zero length body and a `CloseReceiver`

### Resetting a stream

To immediately close a stream for both reading and writing, use reset. This should generally only be used on error; during normal operation, both sides should close instead.

To reset a stream, send a message with a zero length body and a `ResetReceiver` (5) or `ResetInitiator` (6) flag. Reset must immediately close both ends of the stream for both reading and writing. Writing to a stream after it has been reset is a protocol violation. Since reset is generally sent when an error happens, all future reads from a reset stream should return an error (*not* EOF).
To reset a stream, send a message with a zero length body and a `ResetReceiver` (5) or `ResetInitiator` (6) flag.
See [stream resets](../connections/README.md#resets) for a detailed behaviour description.

## Implementation notes

Expand Down