4 ms·

Go channel behaviors are pretty annoying. For one I always forget the panic scenarios (e.g. writing to a closed channel), I feel like the type system could've d

Go channel behaviors are pretty annoying. For one I always forget the panic scenarios (e.g. writing to a closed channel), I feel like the type system could've done more here. I recently wrote a simple function that maps out tasks concurrently, can be canceled by a context.WithCancel, or if a task fails. The things that cancel the task mapper need to coordinate very carefully on both sides of the channel so that they're closed and publishers stop sending in the right sequence. The amount of switches/cancels/signals quickly explode around the coordination if you too cute on how to do it (e.g. read from the error channel to stop the work). Frankly I'm not sure I still got it right [1]. And this is probably the most unsettling part. Rereading the code I can't possibly remember the cancellation semantics and ordering of the short mess I created. Now I'm wondering if mutexes would've made for more understandable code. [1] https://gist.github.com/pnegahdar/1783f0a4e03dc9a3da43478994923956 https://gist.github.com/pnegahdar/1783f0a4e03dc9a3da43478994...

jerf 2y ago
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 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.
- parhamn 2y ago
  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.
  - jerf 2y ago
    "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.
  - deleted 2y ago
    [deleted]
- jrockway 2y ago
  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.
iimblack 2y ago
Might have simplified this with errgroup [0]. [0] https://pkg.go.dev/golang.org/x/sync/errgroup https://pkg.go.dev/golang.org/x/sync/errgroup
dlisboa 2y ago
Not speaking on whether the language should make this easier without an external library, but wouldn't https://github.com/sourcegraph/conc 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.

jerf 2y ago

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 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.

parhamn 2y ago
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.
- jerf 2y ago
  "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.
- deleted 2y ago
  [deleted]
jrockway 2y ago
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.

iimblack 2y ago

Might have simplified this with errgroup [0]. [0] https://pkg.go.dev/golang.org/x/sync/errgroup https://pkg.go.dev/golang.org/x/sync/errgroup

dlisboa 2y ago

Not speaking on whether the language should make this easier without an external library, but wouldn't https://github.com/sourcegraph/conc 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.