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

Full stream close guarantee #656

Closed
vasco-santos opened this issue Jun 4, 2020 · 0 comments · Fixed by #1864
Closed

Full stream close guarantee #656

vasco-santos opened this issue Jun 4, 2020 · 0 comments · Fixed by #1864
Labels
kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked

Comments

@vasco-santos
Copy link
Member

js-libp2p should have an utility logic regarding full stream close, where we wait for an EOF and then reset the stream if it does not close quickly enough.

Some inspiration: https://github.com/libp2p/go-libp2p-core/blob/master/helpers/stream.go#L25-L56

@vasco-santos vasco-santos added kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked labels Jun 4, 2020
achingbrain added a commit that referenced this issue Jul 3, 2023
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async
- The `Connection` interface now has `.close` and `.abort` methods
- `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing
- `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data
- `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and
`@libp2p/webrtc`

Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise`
though they will not block the release as their code is temporarily added to this repo to let CI run.

Fixes #1793
Fixes #656

BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
achingbrain added a commit that referenced this issue Jul 3, 2023
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async
- The `Connection` interface now has `.close` and `.abort` methods
- `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing
- `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data
- `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and
`@libp2p/webrtc`

Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise`
though they will not block the release as their code is temporarily added to this repo to let CI run.

Fixes #1793
Fixes #656

BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
achingbrain added a commit that referenced this issue Jul 20, 2023
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async
- The `Connection` interface now has `.close` and `.abort` methods
- `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing
- `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data
- `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and
`@libp2p/webrtc`

Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise`
though they will not block the release as their code is temporarily added to this repo to let CI run.

Fixes #1793
Fixes #656

BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
achingbrain added a commit that referenced this issue Jul 20, 2023
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async
- The `Connection` interface now has `.close` and `.abort` methods
- `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing
- `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data
- `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and
`@libp2p/webrtc`

Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise`
though they will not block the release as their code is temporarily added to this repo to let CI run.

Fixes #1793
Fixes #656

BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
achingbrain added a commit that referenced this issue Jul 20, 2023
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async
- The `Connection` interface now has `.close` and `.abort` methods
- `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing
- `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data
- `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and
`@libp2p/webrtc`

Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise`
though they will not block the release as their code is temporarily added to this repo to let CI run.

Fixes #1793
Fixes #656

BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
achingbrain added a commit that referenced this issue Jul 20, 2023
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async
- The `Connection` interface now has `.close` and `.abort` methods
- `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing
- `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data
- `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and
`@libp2p/webrtc`

Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise`
though they will not block the release as their code is temporarily added to this repo to let CI run.

Fixes #1793
Fixes #656

BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
achingbrain added a commit that referenced this issue Jul 20, 2023
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async
- The `Connection` interface now has `.close` and `.abort` methods
- `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing
- `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data
- `.reset` is removed from the `Stream` interface - instead call `.abort(err)` to signal a local error
- `.reset` is still present on the `AbstractStream` class - the muxer implementation should call this to signal a remote error
- `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and `@libp2p/webrtc` - all the logic around the \*checks notes* 17 different ways to close a stream is contained there

Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise` though they will not block the release as their code is temporarily added to this repo to let CI run.

Fixes #1793
Fixes #656

BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
This was referenced Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants