The goroutine leak that took three weeks to find (the fix was four lines)

One of our Go services gained about 40 MB of memory a day. Nobody noticed, because we deploy every day and a deploy restarts every pod. Then came a holiday change freeze. Twelve days without a deploy. On day nine the pods started getting OOMKilled, one by one, during the busiest week of the year.

Finding it

The service already imported net/http/pprof on an internal port, which is the single best decision anyone made on that codebase.

go tool pprof http://localhost:6060/debug/pprof/goroutine
(pprof) top

About 180,000 goroutines, almost all of them parked on chan send in the same function. The goroutine count, not the heap, was the story. The heap was just the luggage they were carrying.

The code

func (s *Server) fetchPrice(ctx context.Context, sku string) (Price, error) {
	ctx, cancel := context.WithTimeout(ctx, 300*time.Millisecond)
	defer cancel()

	ch := make(chan result)
	go func() {
		p, err := s.pricing.Get(sku) // note: no ctx
		ch <- result{p, err}
	}()

	select {
	case r := <-ch:
		return r.price, r.err
	case <-ctx.Done():
		return Price{}, ctx.Err()
	}
}

Read it slowly. When the pricing service is slower than 300 ms, the select takes the ctx.Done() branch and the function returns. Nobody will ever read from ch again. The goroutine finishes its HTTP call eventually, tries to send on an unbuffered channel with no receiver, and blocks. Forever. Holding the response, the buffers, everything.

And because s.pricing.Get did not take the context, our "timeout" never cancelled the actual HTTP request either. The caller stopped waiting. The work kept going. A timeout in the caller is not a cancellation of the callee.

The four lines

	ch := make(chan result, 1) // the send can always complete, even if nobody reads
	go func() {
		p, err := s.pricing.Get(ctx, sku) // cancelled when we give up
		ch <- result{p, err}
	}()

And inside the client, the request gets the context:

req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)

With a buffer of one, the goroutine can always drop its result and exit, and the garbage collector cleans up the channel. With the context passed down, the HTTP request is actually aborted when we stop caring about it.

Making sure it stays fixed

• goleak in tests. go.uber.org/goleak, one line in each package: func TestMain(m *testing.M) { goleak.VerifyTestMain(m) }. Any test that leaves a goroutine behind fails. We found four more leaks the first day.
• Alert on go_goroutines. The Prometheus Go client exports it for free. A count that only goes up between deploys is a leak, every time.
• Every goroutine needs an exit. In review we now ask two questions for every go func(): who reads this channel, and what happens if they leave early.

My father ran a dry cleaner for 31 years and never once left a garment on the rack without a ticket. Every goroutine needs a ticket. Three weeks of my life for a lesson he could have taught me in one sentence.