goroutine leak in example of book The Go Programming Language

I am reading The Go Programming Language book, there is an example of the book which demonstrates goroutine leaking

func mirroredQuery() string {
    responses := make(chan string, 3)
    go func() { responses <- request("asia.gopl.io") }()
    go func() { responses <- request("europe.gopl.io") }()
    go func() { responses <- request("americas.gopl.io") }()
    return <-responses // return the quickest response
}
func request(hostname string) (response string) { /* ... */ }

And I have tried to solve the leak, and got the following code

func request(url string) string {
    res, err := http.Get(url)
    if err == nil {
        body, err := io.ReadAll(res.Body)
        if err == nil {
            return string(body)
        } else {
            return err.Error()
        }
    } else {
        return err.Error()
    }
}

func getany() string {
    rsp := make(chan string, 3)
    done := make(chan struct{}, 3)
    doRequest := func(url string) {
        select {
            case rsp <- request(url):
                fmt.Printf("get %s\n", url)
                done <- struct{}{}
            case <- done:
                fmt.Printf("stop %s\n", url)
                return
        }
    }
    go doRequest("http://google.com")
    go doRequest("http://qq.com")
    go doRequest("http://baidu.com")
    return <-rsp
}

but it seems does not solve the problem? any suggestions?

There is no goroutine leakage in the provided code. The mirroredQuery method uses a buffered channel to collect the result and return the first answer. And the currently buffer has enough space to collect all answers from all goroutines, even if the rest of the responses are never read. The situation will change if the buffer is smaller than N - 1, where N is the number of spawned goroutines. In this situation some of the goroutines spawned by mirroredQuery will get stuck trying to send a response to the responses channel. Repeating the call to mirroredQuery will cause increase of stucked goroutines which can be called goroutines leak.

Here is the code with the logs added and the output for both scenarios.

func mirroredQuery() string {
	responses := make(chan string, 2)
	go func() {
		responses <- request("asia.gopl.io")
		log.Printf("Finished goroutine asia.gopl.io\n")
	}()
	go func() {
		responses <- request("europe.gopl.io")
		log.Printf("Finished goroutine europe.gopl.io\n")
	}()
	go func() {
		responses <- request("americas.gopl.io")
		log.Printf("Finished goroutine americas.gopl.io\n")
	}()
	return <-responses // return the quickest response
}
func request(hostname string) (response string) {
	duration := time.Duration(rand.Int63n(5000)) * time.Millisecond
	time.Sleep(duration)
	return hostname
}

func main() {
	rand.Seed(time.Now().UnixNano())
	result := mirroredQuery()
	log.Printf("Fastest result for %s\n", result)
	time.Sleep(6*time.Second)
}

Output for buffer size >= N-1

2021/06/26 16:05:27 Finished europe.gopl.io
2021/06/26 16:05:27 Fastest result for europe.gopl.io
2021/06/26 16:05:28 Finished asia.gopl.io
2021/06/26 16:05:30 Finished americas.gopl.io

Process finished with the exit code 0

Output for buffer size < N-1

2021/06/26 15:47:54 Finished europe.gopl.io
2021/06/26 15:47:54 Fastest result for europe.gopl.io

Process finished with the exit code 0

Above implementation can be "improved" by introducing goroutines termination when the first response arrives. This can potentially lower the number of used resources. It strongly depends on what request method do. For computation heavy scenarios it makes sense, for cancelling http request may lead to a connection termination, so the next request must open new one. For highly loaded servers it may be less effective than waiting for a response even if response is not used.

Below is the improved implementation with context usage.

func mirroredQuery() string {
	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()
	responses := make(chan string)
	f := func(hostname string) {
		response, err := request(ctx, hostname)
		if err != nil {
			log.Printf("Finished %s with error %s\n", hostname, err)
			return
		}
		responses <- response
		log.Printf("Finished %s\n", hostname)
	}
	go f("asia.gopl.io")
	go f("europe.gopl.io")
	go f("americas.gopl.io")
	return <-responses // return the quickest response
}

func request(ctx context.Context, hostname string) (string, error) {
	duration := time.Duration(rand.Int63n(5000)) * time.Millisecond
	after := time.After(duration)
	select {
	case <-ctx.Done():
		return "", ctx.Err()
	case <-after:
		return "response for "+hostname, nil
	}
}

func main() {
	rand.Seed(time.Now().UnixNano())
	result := mirroredQuery()
	log.Printf("Fastest result for %s\n", result)
	time.Sleep(6 * time.Second)
}

You read the book wrong. The book used the example to illustrate how to use a buffered channel to avoid goroutine leak.

This is the paragraph immediately follows the example in the book (page 233):

> Had we used an unbuffered channel, the two slower goroutines would
> have gotten stuck trying to send their responses on a channel from
> which no goroutine will ever receive. This situation, called a
> goroutine leak, would be a bug. Unlike garbage variables, leaked
> goroutines are not automatically collected, so it is important to make
> sure that goroutines terminate themselves when no longer needed.

Note:

1. This function does not try to optimize for memory footprint or resource uses (including network resources). Go's net/http package's client functions are context aware, so it can cancel in middle of a request, which would save some resources (whether that matters for the trouble would be a design decision).

To use context, you can:

func mirroredQuery() string {
responses := make(chan string, 3)
ctx, cf := context.WithCancel(context.Background())
defer cf()
go func() { responses <- request("asia.gopl.io") }()
go func() { responses <- request("europe.gopl.io") }()
go func() { responses <- request("americas.gopl.io") }()
return <-responses // return the quickest response
}
func request(ctx context.Context, url string) string {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
panic(err)
}
res, err := http.DefaultClient.Do(req)
if err == nil {
body, err := io.ReadAll(res.Body)
if err == nil {
return string(body)
} else {
return err.Error()
}
} else {
return err.Error()
}
}

2. Using a buffered channel allocates memory. When there are too many goroutines, it would be too wasteful to use a buffered channel.

To solve this, you can use a channel (like what you attempted):

func getAny() string {
responses := make(chan string)
ctx, cf := context.WithCancel(context.Background())
defer cf()
done := make(chan struct{})
defer close(done)
doRequest := func(url string) {
select {
case responses <- request(ctx, url):
fmt.Printf("get %s\n", url)
case <-done:
fmt.Printf("stop %s\n", url)
return
}
}
go doRequest("http://google.com")
go doRequest("http://qq.com")
go doRequest("http://baidu.com")
return <-responses // return the quickest response
}

Receiving on a closed channel always "returns" a zero value immediately, thus serves as a broadcast. It is common practice to use this kind of "done channel".

You can also use context.Context:

func mirroredQuery() string {
responses := make(chan string)
ctx, cf := context.WithCancel(context.Background())
defer cf()
doRequest := func(url string) {
select {
case responses <- request(ctx, url):
fmt.Printf("get %s\n", url)
case <-ctx.Done():
fmt.Printf("stop %s\n", url)
return
}
}
go doRequest("http://google.com")
go doRequest("http://qq.com")
go doRequest("http://baidu.com")
return <-responses // return the quickest response
}

This is better in this situation in that you already used a context.Context with http.

3. Use a sync.WorkGroup would wait all request to finish but returns the first one. I think that defeats the function's purpose, and provides virtually no benefits. And I don't think making all goroutines the functions spawned returns before the function itself returns makes sense (unless the function is main function).

To avoid leaking goroutines, you presumably want to ensure that once you return from mirroredQuery, no goroutines that were originally created in this function remain running?

In that case, the most important thing is to be able to cancel the other goroutines when one of them manages to do the request successfully. This cancellation is achieved in Go using context.Context, which net/http supports.

Once you have context cancellation in place, you need a sync.WaitGroup in your main function to wait for all goroutines to be Done.

Here's a doRequest that uses a context and wraps the "HTTP get" functionality of the book's request function:

func doRequest(ctx context.Context, url string) string {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
log.Fatal(err)
}
res, err := http.DefaultClient.Do(req)
// err will be non-nil also if the request was canceled
if err != nil {
return ""
}
defer res.Body.Close()
b, err := io.ReadAll(res.Body)
if err != nil {
return ""
}
return string(b)
}

http.DefaultClient.Do will return early if the context is cancelled, with an appropriate error.

Now, the function to juggle the goroutines becomes:

func mirroredQuery() string {
ctx, cancel := context.WithCancel(context.Background())
responses := make(chan string, 3)
fetcher := func(url string, wg *sync.WaitGroup) {
res := doRequest(ctx, url)
if res != "" {
responses <- res
}
wg.Done()
}
urls := []string{
"asia.gopl.io",
"europe.gopl.io",
"http://google.com",
}
var wg sync.WaitGroup
for _, url := range urls {
wg.Add(1)
go fetcher(url, &wg)
}
res := <-responses
fmt.Println("got response", res[:300])
cancel()
wg.Wait()
return res
}

Note a few things:

• Each goroutine runs doRequest and only writes the result to responses if the result is non-empty (meaning no error occurred; cancel counts as an error here)
• The WaitGroup is used to wait for all worker goroutines to exit
• The main goroutine launches all workers then waits for the first (non-empty) result in responses; it then calls cancel to cancel the context which signals all the worker goroutines to exit, and waits for them to be done.

As an exercise, extend this code to solve a few issues:

• Distinguish between real errors and cancellation; in the current code there could be a deadlock if all workers run into an error
• Add a timeout to the <- responses read in the main goroutine using select.
• Write code to return the first result ASAP to the caller, while a background goroutine can deal with canceling the context and waiting for the workers to exit. After all, the primary goal here is to return the result fast.

Use context and sync.WaitGroup