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

Fix select with receive? and closed channels #8304

Merged
merged 13 commits into from Oct 11, 2019

Conversation

@bcardiff
Copy link
Member

bcardiff commented Oct 10, 2019

This PR implements https://gist.github.com/bcardiff/289953a80eb3a0512a2a2f8c8dfeb1db while fixing the behavior of select over closed or closing channels.

The main guidance where:

  • Channel#receive vs Channel#receive? differs on their behavior for closed channels. Using them directly is always blocking.
  • Performing an operation over a closed and closing the channel during the operation must behave in the same way.

There was a need to extend the design of SelectAction and some overload/exceptions were used used to make typing work.
Regarding typing, blocking and non-blocking select have different methods so their typing can differ. Blocking select will never return NotReady.

Closes #8301
Fixes #8300
Follow up of #8243 (comment)

@bcardiff bcardiff requested a review from waj Oct 10, 2019
src/channel.cr Outdated Show resolved Hide resolved
@bcardiff bcardiff force-pushed the bcardiff:mt/select-closed-channel branch from c1cd9ef to afb5357 Oct 11, 2019
@bcardiff

This comment has been minimized.

Copy link
Member Author

bcardiff commented Oct 11, 2019

squashed fixups and rebased on master after today's review to improve the helper methods.

@waj
waj approved these changes Oct 11, 2019
@bcardiff bcardiff merged commit 8463046 into crystal-lang:master Oct 11, 2019
6 checks passed
6 checks passed
ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32_std Your tests passed on CircleCI!
Details
ci/circleci: test_preview_mt Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bcardiff bcardiff deleted the bcardiff:mt/select-closed-channel branch Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.