go: proposal: Go 2: sends on closed channels do not panic

Motivation

A common pattern I’ve seen is using a lock to protect a channel and a variable which indicates whether or not the channel is closed. For example:

type Queue struct {
  sync.RWMutex

  closed bool
  ch chan T
}

func New(cap int) *Queue {
  return &Queue{ ch: make(chan T, cap) } 
}

func (q *Queue) Enqueue(t T) error {
  q.RLock()
  defer q.RUnlock()
  if q.closed {
    return errors.New("queue is closed")
  }
  q.ch <- t
}

func (q *Queue) Close() {
  q.Lock()
  defer q.Unlock()
  if q.closed {
    return
  }
  close(q.closed)
  q.closed = true
}

Using this approach one has to acquire two locks in each call to Enqueue. The first lock being the read lock for q.closed and the second being the internal lock used by the channel. However, the channel already has a field closed to indicate whether the channel has been closed and a lock which protects it, so the extra lock is only necessary because there is no publicly accessible API for determining if the channel is closed.

Proposal

To support cases like the one above, where a channel may be closed concurrently with sends on it, I’d like to propose that Go add support for conditionally sending values on a channel by returning a bool from send operations to indicate whether the send was successful. Sends to a channel could then do the following:

 if ok := q.ch <- t; !ok {
  return errChanClosed
}

Callers that do, in fact, want the send to panic could explicitly call panic themselves. While those that want to treat such a case as an error will have that ability as well.

Thanks!

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 34
Comments: 31 (9 by maintainers)

Most upvoted comments

Also, let’s make integer division by zero silently return a random number. /s

+29

cznic on Sep 23, 2017

Under this proposal, incorrect code that broke before will silently fail, that is, appear to run correctly while still being incorrect. That seems retrograde.

+18

robpike on Sep 23, 2017

you can use an extra channel

Sure, I can make the structure of my code increasingly complex. Or I could simply write

ch, ok <- &Request{...}
if !ok {
    return 0, ErrDataUnavailable
}

in the reader passed to http.ServeContents.

jech on Sep 23, 2017

If a program sends on a closed channel, it’s not managing resources correctly. You could argue that your proposal makes it easier to do that, but I would counter that it makes it too easy to construct poor concurrent structures.

robpike on Sep 23, 2017

I wrote previously:

ch, ok <- &Request{...}
if !ok {
    return 0, ErrDataUnavailable
}

Looking at this some more, there’s a race condition: if the (asynchronous) channel is closed just after the request is written, the request is discarded with no error indication to the caller.

So I’m slowly starting to feel that @robpike is right — this proposal would lead to bad code.

jech on Sep 25, 2017

Looking at this some more, there’s a race condition: if the (asynchronous) channel is closed just after the request is written, the request is discarded with no error indication to the caller.

Wouldn’t that be equivalent to closing a buffered channel that has values in it though? The values are still in the channel and available to be pulled off.

jeromefroe on Sep 25, 2017

…despite your protests…

your proposal makes it too easy to write bad concurrent structures.

Just to avoid any blame getting misdirected – there’s two bodies here. The proposal is due to @jeromefroe, who remained impeccably polite throughout the discussion. It’s only me that’s rudely protesting.

jech on Sep 24, 2017

As others have pointed out, the language already has all the tools you need to achieve your goal, and despite your protests it’s really not much more complex to use them.

And as I said before, your proposal makes it too easy to write bad concurrent structures. It’s analogous to how reentrant locks result in bad locking protocols.

robpike on Sep 24, 2017

how about just like type assertion, if ok is present, then not panic, otherwise, panic just like Go 1.

go101 on Sep 23, 2017