Skip to content

Potential Goroutine Leak Due to Unbuffered Channel #16665

Description

@CertiK-Geth

Locations

Description

The Do function from cl/sentinel/httpreqresp/server.go initiates a goroutine that sends results to an unbuffered channel ans. If the r.Context().Done() branch wins the race condition, then the goroutine running handler.ServeHTTP() will still be alive and blocked on ans <- resp because nothing is receiving from ans channel, the goroutine will block indefinitely, leading to a resource leak. Below is the relevant code snippet:

func Do(handler http.Handler, r *http.Request) (*http.Response, error) {
	// TODO: there potentially extra alloc here (responses are bufferd)
	// is that a big deal? not sure. maybe can reuse these buffers since they are read once (and known when close) if so
	ans := make(chan *http.Response)
	go func() {
		res := httptest.NewRecorder()
		handler.ServeHTTP(res, r)
		// linter does not know we are passing the resposne through channel.
		// nolint: bodyclose
		resp := res.Result()
		ans <- resp
	}()
	select {
	case res := <-ans:
		return res, nil
	case <-r.Context().Done():
		return nil, r.Context().Err()
	}
}

Recommendation

To prevent the goroutine from blocking indefinitely, make the ans channel buffered. This allows the goroutine to send a response without blocking, even if the r.Context().Done() branch is selected first. Modify the channel declaration as follows:

ans := make(chan *http.Response, 1)

Metadata

Metadata

Assignees

Labels

CaplinCaplin: Consensus Layer, Beacon API

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions