Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

add CloseRead/CloseWrite on streams #166

Merged
merged 2 commits into from
Sep 2, 2020
Merged

add CloseRead/CloseWrite on streams #166

merged 2 commits into from
Sep 2, 2020

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Aug 27, 2020

This changes the behavior of Close to behave as one would expect: it closes
the stream. The new methods, CloseWrite/CloseRead allow for closing the stream in
a single direction.

Note: This does not implement CancelWrite/CancelRead as our stream muxer
protocols don't support that.

fixes #9
followup from #115, which is a followup from #10.

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

At last! It will be quite an endeavor to update our code for this however.

@Stebalien
Copy link
Member Author

At last! It will be quite an endeavor to update our code for this however.

I've already written the patch for yamux and I'm currently writing the patch for mplex. It's actually pretty easy to update code using streams, because pretty much all of it either:

  1. Uses FullClose and AwaitEOF, which have now been removed.
  2. Uses Reset (no change).
  3. Uses Close incorrectly (which will now become "correctly").

Stebalien added a commit to libp2p/go-mplex that referenced this pull request Aug 28, 2020
Close now closes the stream in both directions while CloseRead discards inbound
bytes and CloseWrite sends an EOF. This matches the user's expectation where
Close actually closes the stream.

part of libp2p/go-libp2p-core#166
This changes the behavior of `Close` to behave as one would expect: it closes
the stream. The new methods, CloseWrite/CloseRead allow for closing the stream in
a single direction.

Note: This _does not_ implement CancelWrite/CancelRead as our stream muxer
_protocols_ don't support that.

fixes #9
FullClose and AwaitEOF were introduced to work around the fact that calling
Close on a stream only closed the write half. All users must adapt their code
to the new interfaces, so this change is intentionally breaking.
Stebalien added a commit to libp2p/go-libp2p-quic-transport that referenced this pull request Aug 28, 2020
This:

* Changes `Close` to behave like the unix `close` (sends an EOF, ignores future
inbound data).
* Adds a `CloseWrite` to replace the current `Close`.
* Adds a `CloseRead` to close the read side, while leaving the write side open.

See libp2p/go-libp2p-core#166.
Stebalien added a commit to libp2p/go-mplex that referenced this pull request Aug 28, 2020
Close now closes the stream in both directions while CloseRead discards inbound
bytes and CloseWrite sends an EOF. This matches the user's expectation where
Close actually closes the stream.

part of libp2p/go-libp2p-core#166
Stebalien added a commit to libp2p/go-mplex that referenced this pull request Aug 28, 2020
Close now closes the stream in both directions while CloseRead discards inbound
bytes and CloseWrite sends an EOF. This matches the user's expectation where
Close actually closes the stream.

part of libp2p/go-libp2p-core#166
@Stebalien
Copy link
Member Author

The test failures here are just warnings about breaking changes. All muxer PRs are ready to review.

@Stebalien Stebalien merged commit d6afc69 into master Sep 2, 2020
@Stebalien Stebalien deleted the feat/rw-close2 branch September 2, 2020 00:30
Stebalien added a commit to libp2p/go-libp2p-quic-transport that referenced this pull request Sep 2, 2020
This:

* Changes `Close` to behave like the unix `close` (sends an EOF, ignores future
inbound data).
* Adds a `CloseWrite` to replace the current `Close`.
* Adds a `CloseRead` to close the read side, while leaving the write side open.

See libp2p/go-libp2p-core#166.
moul-bot pushed a commit to berty/berty that referenced this pull request Nov 13, 2020
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
moul-bot pushed a commit to berty/berty that referenced this pull request Nov 13, 2020
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@aschmahmann aschmahmann mentioned this pull request Feb 18, 2021
73 tasks
marten-seemann pushed a commit to libp2p/go-libp2p that referenced this pull request Apr 22, 2022
This:

* Changes `Close` to behave like the unix `close` (sends an EOF, ignores future
inbound data).
* Adds a `CloseWrite` to replace the current `Close`.
* Adds a `CloseRead` to close the read side, while leaving the write side open.

See libp2p/go-libp2p-core#166.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: Why doesn't Close() close the stream?
3 participants