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

Connection/Channel.NotifyClose: allow only buffered channel=1 #256

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ikavgo
Copy link

@ikavgo ikavgo commented Apr 9, 2024

Copy link
Contributor

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

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

Do we want to force the buffer size to be 1? What's wrong with higher buffer sizes?

I had a in mind a check to ensure that we don't use unbuffered channels i.e. cap(ch) != 0. What's the reasoning to enforce the capacity to always be one?

I'm unsure if we want to panic either, it seems a bit extreme to crash the entire program just because the argument of a library function was incorrect. I think we should look for alternatives to introduce this change, for example:

  • Create a new function that returns a channel and an error, and return a non-nil error if channel is buffered; and deprecate this function.
  • Use a channel of capacity 1 if an unbuffered channel is passed (bit ugly), else use the channel as-is.
  • Any other approach than panicking is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants