Skip to content

Conversation

ronag
Copy link
Member

@ronag ronag commented Mar 14, 2020

net.Socket is slightly breaking stream invariants by having readable/writable going from false to true. Streams assume that readable/writable starts out true and then goes to false through push(null)/end() after which it never goes back to true, e.g. once a stream is writable == false it is assumed it will never become true.

This PR changes 2 things:

Unless explicitly set to false through options:

  • starts as readable/writable true by default.
  • uses push(null)/end() to set readable/writable to false. Note that this would cause the socket to emit the 'end'/'finish' events, which it did not do previously.

In the case it is explicitly set to false through options it is assumed to never become true.

This makes net behave like regular streams and the way quic currently handles the corresponding functionality.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@ronag ronag added net Issues and PRs related to the net subsystem. stream Issues and PRs related to the stream subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Mar 14, 2020
@ronag ronag requested a review from lpinca March 14, 2020 21:27
@ronag ronag force-pushed the net-stream-writable-readable branch from d810373 to 0866ef1 Compare March 14, 2020 21:46
@ronag ronag added the wip Issues and PRs that are still a work in progress. label Mar 14, 2020
@ronag ronag removed the request for review from lpinca March 14, 2020 22:04
@ronag

This comment has been minimized.

@ronag ronag closed this Mar 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

net Issues and PRs related to the net subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stream Issues and PRs related to the stream subsystem. wip Issues and PRs that are still a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant