Hacker News Reader

Writing to a closed channel at all is generally a design smell. Generally this is consumers closing channels, which is not a good idea. Only producers should close channels, generally only a producer that is the sole owner of the channel, and then, being the sole owner, it should "know" that it closed the channel and by its structure never write to it again. You probably are overcomplicating matters and need just one top-level channel, which in modern Go is the one contained in a context.Context, to be the one and only stop signal in the system.

I think everyone goes through a bit of a complication phase with channels, I recognize your issues in code I've written myself, this is definitely not a "only a bad person would have this problem" post. But, yes, there probably is an organization that solves this problem. There's an art to using them properly. A good 50% of that art may well be that consumers should never close channels.

(Another one is the utility of sending a channel as part of the message in a different channel. It is intuitively easy to think that channels must be very expensive, but when they're not participating in a select, they're just a handful of machine words in RAM. It is perfectly sensible to send a message to a "server" process that contains the channel in it to send the reply to, because it only costs a few machine words in allocation and precisely the one sync operation it will ever participate in. Channels do not have to amortize their costs with lots of messages; a 1-message channel is practical. This also cleaned up some complicated code I had before, trying to prematurely optimize something that was already very cheap.)

The other thing is, if you haven't looked at https://pkg.go.dev/golang.org/x/sync/errgroup , you may want to. golang.org/x/ is the not-as-well-known-as-it-should-be extended standard library; things the Go team are not willing to put into the 1.0 backwards compatibility promise so they retain the ability to change things if necessary, but otherwise de facto as high quality as the standard library, modulo some reasonably well-labeled exceptions. Contra some claims that it is impossible to abstract in Go, many of these common concurrency patterns have been abstracted out and you can and should grab them off the shelf.

People are missing a key insight about channels, which is that close is a write. Typically, you want one piece of code reading, and another piece of code writing, and that's where their code goes wrong. A panic is appropriate in this case, as it's always a bug.

The underlying feature that people are looking for is a TCP-like "either side can kill the other side" feature. This is what contexts are.

   func read(ctx context.Context, c <-chan any) {
       for {
           select {
           case <-ctx.Done():
               return
           case x, ok := <-c:
               if ok {
                   fmt.Println(x)
               } else {
                   return
               }
           }
       }
   }

   func write(ctx context.Context, c chan<- any) {
       for i := 0; i < 10; i++ {
           select {
           case <-ctx.Done():
               close(c)
               return
           case c <- i:
           }
       }
   }

In this example, the type system prevents the read function from writing to the channel (avoiding the panic in the writer if the reader were to close the channel), and the context causes both sides to exit cleanly. You can plumb in the cancel function returned from context.WithCancel/context.WithCancelCause to allow the reader to kill the writer. (Incidentally, the reason you have to do this is because context.Done() is type <-chan, not type chan, so you can't just close(ctx.Done()).)

The writer can still kill the reader by closing the channel, because closes are writes.

More importantly close is a _broadcasting_ write. Which is an incredibly useful feature when you want to build nested structures out of your channel paths.

Yup! This is exactly how context.WithCancel is implemented; <-ctx.Done() is the signal that everything that cares about cancellation listens on. The returned CancelFunc just does `close(thatChannel)`.

I prefer explicitly using contexts for cancellation, but in a pinch:

   c := make(chan struct{})
   foo(c)
   bar(c)
   close(c)

is a useful pattern. (I do this a lot in tests that involve concurrency; the test calls t.Cleanup(func() { close(c) }) and then if the test fails with t.Fatal or something, all background work is killed.)

Exactly right. These "done channels" were extremely widely used as a doneCh parameter before contexts were introduced in 1.7.

x/sync is great. I use things like singlelfight frequently.

> Generally this is consumers closing channels, which is not a good idea.

I've heard this too. I updated my comment to include the code snippet. But any general tips on when the consumer needs to cancel the rest of the work because of an error?

I don't recall now but I think I ended up writing my own for fast-fail and generic support. Really hoping the x/sync packages get generic support soon!

> a 1-message channel is practical

Great tip, every time I add a little signal one I feel like I did something wrong, maybe that is the move.

"But any general tips on when the consumer needs to cancel the rest of the work because of an error?"

Yes, another channel which everyone watches for cancellation.

In modern Go, that should be a context and it's .Done() channel.

This is another "I'm not saying you suck because I did this myself a while before I figured it out", but having a channel around just to cancel things is fine. It doesn't cost anything significantly extra. I have some still in my code bases where I've just never had any reason to upgrade to contexts.

Not speaking on whether the language should make this easier without an external library, but wouldn't https://github.com/sourcegraph/conc help in that scenario? It has context-aware and error-aware goroutine pools, seems like the exact fit for what you are trying to do. Although admittedly I dive too deep into your code.

Might have simplified this with errgroup [0].

[0] https://pkg.go.dev/golang.org/x/sync/errgroup