Why goroutine leaks

Question

I read Twelve Go Best Practices and encounter and interesting example on page 30.

func sendMsg(msg, addr string) error {
    conn, err := net.Dial("tcp", addr)
    if err != nil {
        return err
    }
    defer conn.Close()
    _, err = fmt.Fprint(conn, msg)
    return err
} 

func broadcastMsg(msg string, addrs []string) error {
    errc := make(chan error)
    for _, addr := range addrs {
        go func(addr string) {
            errc <- sendMsg(msg, addr)
            fmt.Println("done")
        }(addr)
    }

    for _ = range addrs {
        if err := <-errc; err != nil {
            return err
        }
    }
    return nil
}

func main() {
    addr := []string{"localhost:8080", "http://google.com"}
    err := broadcastMsg("hi", addr)

    time.Sleep(time.Second)

    if err != nil {
        fmt.Println(err)
        return
    }
    fmt.Println("everything went fine")
}

The programmer mentioned, that happens to the code above:

the goroutine is blocked on the chan write
the goroutine holds a reference to the chan
the chan will never be garbage collected

Why the goroutine is blocked here? The main thread is blocked, until it receive data from goroutine. After it continues the for loop. Not?

Why the errc chan will be never garbage collected? Because I do not close the channel, after goroutine is finished?

Answer 1

One problem I see is that inside broadcastMsg() after goroutines have started:

for _ = range addrs {
    if err := <-errc; err != nil {
        return err
    }
}

If a non-nil error is received from errc, broadcastMsg() returns immediately with that error and does not receive futher values from the channel, which means further goroutines will never get unblocked because errc is unbuffered.

Possible Fixes

A possible fix would be to use a buffered channel, big enough to not block any of the goroutines, in this case:

errc := make(chan error, len(addrs))

Or even if a non-nil error is received from the channel, still proceed to receive as many times as many goroutines send on it:

var errRec error
for _ = range addrs {
    if err := <-errc; err != nil {
        if errRec == nil {
            errRec = err
        }
    }
}
return errRec

Or as mentioned in the linked talk on slide #33: use a "quit" channel to prevent the started goroutines to remain blocked after broadcastMsg() has completed/returned.

Answer 2

You have a list of two addresses (localhost, google). To each of these you're sending a message (hi), using one goroutine per address. And the goroutine sends error (which may be nil) to errc channel.

If you send something to a channel, you also need something that reads the values from that channel, otherwise it will block (unless it's a buffered channel, but even buffered channels block once their buffer is full).

So your reading loop looks like this:

for _ = range addrs {
    if err := <-errc; err != nil {
        return err
    }
}

If the first address returns an error which is not nil, the loop returns. The subsequent error values are never read from the channel thus it blocks.