Is there a resource leak here?

Question

func First(query string, replicas ...Search) Result {
  c := make(chan Result)
  searchReplica := func(i int) {
    c <- replicas[i](query)
  }
  for i := range replicas {
    go searchReplica(i)
  }
  return <-c
}

This function is from the slides of Rob Pike on go concurrency patterns in 2012. I think there is a resource leak in this function. As the function return after the first send & receive pair happens on channel c, the other go routines try to send on channel c. So there is a resource leak here. Anyone knows golang well can confirm this? And how can I detect this leak using what kind of golang tooling?

Answer 1

Yes, you are right (for reference, here's the link to the slide). In the above code only one launched goroutine will terminate, the rest will hang on attempting to send on channel c.

Detailing:

• c is an unbuffered channel
• there is only a single receive operation, in the return statement
• A new goroutine is launched for each element of replicas
• each launched goroutine sends a value on channel c
• since there is only 1 receive from it, one goroutine will be able to send a value on it, the rest will block forever

Note that depending on the number of elements of replicas (which is len(replicas)):

• if it's 0: First() would block forever (no one sends anything on c)
• if it's 1: would work as expected
• if it's > 1: then it leaks resources

The following modified version will not leak goroutines, by using a non-blocking send (with the help of select with default branch):

searchReplica := func(i int) {
    select {
    case c <- replicas[i](query):
    default:
    }
}

The first goroutine ready with the result will send it on channel c which will be received by the goroutine running First(), in the return statement. All other goroutines when they have the result will attempt to send on the channel, and "seeing" that it's not ready (send would block because nobody is ready to receive from it), the default branch will be chosen, and thus the goroutine will end normally.

Another way to fix it would be to use a buffered channel:

c := make(chan Result, len(replicas))

And this way the send operations would not block. And of course only one (the first sent) value will be received from the channel and returned.

Note that the solution with any of the above fixes would still block if len(replicas) is 0. To avoid that, First() should check this explicitly, e.g.:

func First(query string, replicas ...Search) Result {
    if len(replicas) == 0 {
        return Result{}
    }
    // ...rest of the code...
}

Some tools / resources to detect leaks:

• https://github.com/fortytw2/leaktest
• https://github.com/zimmski/go-leak
• https://medium.com/golangspec/goroutine-leak-400063aef468
• https://blog.minio.io/debugging-go-routine-leaks-a1220142d32c