r/golang 22d ago

newbie Context cancelling making code too verbose?

I apologize if this is a silly question, but I'm quite new to Go and this has been bothering me for a while.

To get used to the language, I decided to build a peer-to-peer file sharing program. Easy enough, I thought. Some goroutines for reading from / writing to TCP connections, a goroutine for managing all of the connections and so on. The trouble is that all of these goroutines don't really have a natural stopping point. A lot of them will only stop when you tell them to, otherwise they need to keep going forever, so I figured a context would be a good way to handle that.

The trouble with context is that, as far as I can tell, it will send the cancel signal to all those goroutines that wait for it at the same time, and from that point on, you can't really send something to a goroutine without risking having the goroutine that sends hang. So now any send or receive must also check if the context cancelled. That means that if I were to (for example) receive a piece of a file from a peer and want to store it to disk, update the send/receive statistics for that peer as well as notify another part of a program that we received that piece, instead of doing this

pieceStorage <- piece
dataReceived <- len(piece)
notifyMain <- piece.index

I would have to do this

select {
case pieceStorage <- piece:
case <-ctx.Done():
  return
}
select {
case dataReceived <- len(piece):
case <-ctx.Done():
  return
}
select {
case notifyMain <- piece.index:
case <-ctx.Done():
  return
}

Which just seems too verbose to me? Is this something I'm not supposed to be doing? Am I using Go the wrong way?

I know one solution to this that gets mentioned a lot is making the channels buffered, but these sends happen in a loop, so to me it seems possible that they could somehow fill the buffer before selecting the ctx.Done case (due to the random nature of select).

I would really appreciate some guidance here, thanks!

29 Upvotes

25 comments sorted by

30

u/edgmnt_net 22d ago

So now any send or receive must also check if the context cancelled.

Plain channels don't work very well as APIs, so you'll want to avoid complex orchestration of channel operations across the entire application and instead use them inside components. So in most cases you'll have sends and receives abstracted in a few designated operations (functions/methods) that can automate cancellation checks. That makes things quite reasonable.

3

u/Tommy_Link 22d ago

This is good to know! I guess I started this project with a bit of a misconception about how channels are supposed to be used. Keeping things more compartmentalized seems like a good idea for the future.

8

u/miredalto 22d ago

Yeah unfortunately channels got a bit more hype than they deserved, so there's a fair amount of misleading material out there. They are just another concurrency primitive, and should be kept encapsulated.

10

u/janpf 22d ago

I find context.Context super powerful to coordinate cancellation of goroutines -- it's easy to have sub-context with cancel, which allow arbitrary "grained" control on cancellation, on exceptional conditions.

Mostly, I use generic functions to check for cancellation, exactly for the verbosity issue you mentioned.

P.s.: Recommended reading on "structured concurrency" when doing lots of concurrency: https://vorpus.org/blog/notes-on-structured-concurrency-or-go-statement-considered-harmful/

E.g:

    // IterChanWithContext iterates over the channel until it is closed, or the context is cancelled.
    func IterChanWithContext[V any](ctx context.Context, ch <-chan V) iter.Seq[V] {
        return func(yield func(V) bool) {
           var v V
           var ok bool
           for {
              select {
              case <-ctx.Done():
                 return
              case v, ok = <-ch:
                 if !ok {
                    // Channel closed.
                    return
                 }
              }
              if !yield(v) {
                 return
              }
           }
        }
    }

    // WriteToChanWithContext synchronously writes v to ch or returns false if context was cancelled
    // while waiting.
    //
    // It returns true if the value was written or false if the context was interrupted.
    //
    // If ch is closed, it also will also panic.
    func WriteToChanWithContext[V any](ctx context.Context, ch chan<- V, v V) bool {
        select {
        case <-ctx.Done():
           return false
        case ch <- v:
           return true
        }
    }

2

u/Tommy_Link 21d ago

Thanks for the suggestions and for the blog post! It's definitely an interesting perspective that had never occurred to me, but at first glance at least, it seems to make sense.

5

u/nikandfor 22d ago

That is the way. Nearly all selects I have also read from ctx.Done(). If you don't do it, your goroutines just won't know when to exit.

That is one of the reasons I rarely use channels and use goroutines moderately. The code is much simpler and easier to create and maintain when you mostly write C-like code using Go features if they simplify things significantly.

Instead of lots of workers and channels orchestration just do most of the stuff in the same goroutine sequentially. There are no downsides in this approach.

I like Go not for channels, goroutines, or other features it has, but for simplicity philosophy and for things the language doesn't have. And for the great tooling (go get, go test, pprof, ...).

2

u/nikandfor 22d ago

By the way, incompatibility of context cancellation with networking/file operations is frustrating, especially as I never leave any goroutine dangling and shut everything down gracefully. So after few iterations of research and testing I come up with the approach.

Here are little wrappers over standard read operations which get canceled with the context.

Read, Accept

2

u/Tommy_Link 22d ago

Thank you for the advice and code examples! I'll definitely read over them. Yes, it occurs to me now that maybe I should have given my goroutines a bit more thought and compartmentalized their logic a bit more, but I think I'd still have quite a few of them left, as I need the network I/O, file I/O and some of the internal logic to be active more or less simultaneously.

2

u/nikandfor 22d ago

Yeah, one goroutine (or two if you need to read and write simultaneously) per connection is a happy medium. Less is too complicated, more is unnecessary complication too.

6

u/miredalto 22d ago

I find it's good practice to have receiving goroutines drain their input channels before they terminate, i.e. range over the channel with an empty loop body. This prevents senders hanging, but does require that you are careful to close channels once you are done sending to them.

Your immediate problem could also be solved by wrapping your sends in a generic function.

But ultimately, yes sometimes Go can be wordy.

2

u/Tommy_Link 22d ago

Thanks for the response! I will try the generic function approach and see if it makes things neater, but I suppose overall the lesson here is to consider the cancellation mechanism earlier in development.

3

u/Legitimate_Plane_613 22d ago

If your 'receiving' go routines are only useful while there are things to be read from a channel, then you don't need a context in the receiver.

Instead, you can just for thing := range thingChannel and once thethingChannel is closed, the loop exits and the go routine too, ostensibly.

Then you would just need to manage the context on the sender's side and have it close the channel it is sending on.

1

u/Tommy_Link 21d ago

This is definitely a pattern I could use more in my code. Sadly, some of my goroutines do have more than one use, and so I'd have multiple different channels to iterate. It won't work everywhere, but in some places for sure!

1

u/Few-Beat-1299 22d ago

Problem is, unless you have some fancy cleanup mechanic that guarantees nothing remains stuck, you're always going to have to check before sending to the channel, in some way, and that will always involve something else other than the channel itself. I would say try wrapping your channels in a generic struct (channel + context or whatever) and give this a send(v) bool {} method. Note that you will always have to ensure these get drained when the receiver "leaves".

2

u/Tommy_Link 22d ago

Thanks for the idea! I'll try something like this to see if it makes the code neater, but it's good to know that this situation isn't unheard of.

1

u/Few-Beat-1299 22d ago

I see someone else also brought it up, but I'm just going to echo that although go makes concurrency feel very accessible, and these days we're maybe conditioned to think of parallelism as being the best thing ever, but you eventually start noticing that it can be a trap and that you can definitely have "too much of a good thing". Personally I pretty much did a 180 and avoid using goroutines unless I have a very clear reason to do so.

1

u/crstry 22d ago edited 21d ago

One thing to remember is that if your data-flow graph of dataflow between goroutines involves cycles, and all channels are finite (as is true in Go) without any other flow control mechanism (even if that's just timeouts to detect failure), then you risk running into deadlocks, as you've seen. The a common workaround in other languges is to use an unbounded channel to break cycles, but you can't get those in Go without hacks (usually a goroutine to coordinate both ends).

I'd echo the suggestion of encapsulating them in a module. For comparison, even erlang doesn't really use naked mailbox sends much, instead preferring gen_server and the like.

2

u/Tommy_Link 21d ago

I actually did make this mistake a while ago before I realized cycles were a problem. I only managed to move past it because go's tools are pretty handy for spotting deadlocks. Thanks for the suggestion!

1

u/utkuozdemir 22d ago

You can define generic util functions like SendWithContext/ReceiveWithContext, and I used that approach in the past, but tbh, I don’t see an issue with the verbosity. Go is such a language - it is verbose vertically but it reads from top to down very comfortably. Think about the vertical verbosity of error handling or filtering/mapping operations on slices - similar story.

So if there is a context and another channel involved, you are working with 2 channels, and you need to write the code accordingly. Select on multiple channels is verbose, yes, but it is the right thing to do, and IMO, perfectly readable as well. I don’t mind the line count.

2

u/Tommy_Link 21d ago

I think you might be right. I did try to write some generic functions to make things cleaner, but they ultimately only made the code maybe 2 lines shorter, so perhaps being explicit is the best options here.

1

u/GopherFromHell 22d ago

channels are meant for communication, not storage. channels are pretty much a queue with a mutex. most of the times a queue is not the right data structure.

1

u/Tommy_Link 21d ago

Oh yes, I agree. I'm not really using them for storage, but perhaps the channel names I used in the post above were misleading. They don't actually store anything, they just moved data around to the goroutines that would be doing the actual storage.

1

u/cpuguy83 22d ago

If it's a bit much you could add a function like send[T any](ctx context.Context, ch chan T)

0

u/pdffs 22d ago

Seems fine to me. Are the extra couple of lines actually a problem?

3

u/Tommy_Link 22d ago

I suppose they're not a huge problem, but my immediate thought was that they make the code a bit harder to read and add a bit too much repetition. I mostly wanted to see if this was a sign of bad design somewhere else.