Why goroutine leaks

Issue

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?

Solution

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.

Answered By – icza

Answer Checked By – Candace Johnson (GoLangFix Volunteer)

Leave a Reply

Your email address will not be published.