Skip to content
This repository was archived by the owner on Oct 28, 2024. It is now read-only.

Avoid an open Peer with a closed Client #65

Merged
merged 1 commit into from
Aug 11, 2020
Merged

Conversation

natebosch
Copy link
Contributor

May fix dart-lang/sdk#43012

If a Peer is created with a StreamChannel that does not follow the
stated contract it's possible that the sink gets closed without
receiving a done event from the channel which leaves the Peer
instance in a state that's inconsistent with the underlying Client.
The result is that it's possible to get a bad state trying to send a
message even with isClosed returns false.

  • Make isClosed and done forward to the _client and _peer fields
    so that they can't be inconsistent.
  • Forward errors to the _server so that it can forward them through
    done without an extra Completer to manage.
  • Avoid closing the sink in the Peer. It will end up being closed by
    the server when it is handling the error, and it's the same sink
    instance in both places.
  • Add a test that ensures that isClosed behaves as expected following
    a call to close() even when the StreamChannel does not follow it's
    contract.

May fix dart-lang/sdk#43012

If a `Peer` is created with a `StreamChannel` that does not follow the
stated contract it's possible that the `sink` gets closed without
receiving a done event from the `channel` which leaves the `Peer`
instance in a state that's inconsistent with the underlying `Client`.
The result is that it's possible to get a bad state trying to send a
message even with `isClosed` returns `false`.

- Make `isClosed` and `done` forward to the `_client` and `_peer` fields
  so that they can't be inconsistent.
- Forward errors to the `_server` so that it can forward them through
  `done` without an extra `Completer` to manage.
- Avoid closing the `sink` in the `Peer`. It will end up being closed by
  the server when it is handling the error, and it's the same `sink`
  instance in both places.
- Add a test that ensures that `isClosed` behaves as expected following
  a call to `close()` even when the `StreamChannel` does not follow it's
  contract.
@natebosch natebosch merged commit 995611c into master Aug 11, 2020
@natebosch natebosch deleted the closed-with-bad-channel branch August 11, 2020 23:23
mosuem pushed a commit to dart-lang/tools that referenced this pull request Oct 25, 2024
May fix dart-lang/sdk#43012

If a `Peer` is created with a `StreamChannel` that does not follow the
stated contract it's possible that the `sink` gets closed without
receiving a done event from the `channel` which leaves the `Peer`
instance in a state that's inconsistent with the underlying `Client`.
The result is that it's possible to get a bad state trying to send a
message even with `isClosed` returns `false`.

- Make `isClosed` and `done` forward to the `_client` and `_peer` fields
  so that they can't be inconsistent.
- Forward errors to the `_server` so that it can forward them through
  `done` without an extra `Completer` to manage.
- Avoid closing the `sink` in the `Peer`. It will end up being closed by
  the server when it is handling the error, and it's the same `sink`
  instance in both places.
- Add a test that ensures that `isClosed` behaves as expected following
  a call to `close()` even when the `StreamChannel` does not follow it's
  contract.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

Failures on Update to latest package:json_rpc_2
3 participants