Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Goroutine leak in timeout #35

Open
peter-crist opened this issue Jan 24, 2022 · 1 comment
Open

Goroutine leak in timeout #35

peter-crist opened this issue Jan 24, 2022 · 1 comment

Comments

@peter-crist
Copy link

In timeout.go, context cancellation isn't check for when running the command goroutine. This causes the errc <- next.Run(ctx, f) call to block indefinitely if the command times out.

ctx, _ = context.WithTimeout(ctx, cfg.Timeout)

// Run the command
errc := make(chan error)
go func() {
	errc <- next.Run(ctx, f) //This blocks indefinitely if the context is cancelled due to a timeout.
}()

// Wait until the deadline has been reached or we have a result.
select {
// Finished correctly.
case err := <-errc:
	return err
// Timeout.
case <-ctx.Done():
	metricsRecorder.IncTimeout()
	return errors.ErrTimeout
}

The simple fix is to select:

go func() {
	select {
	case <-ctx.Done():
	case errc <- next.Run(ctx, f):
	}
}()
@eugennicoara
Copy link

Maybe this fix would have helped me avoid this OOM:

goroutine profile: total 238162
236204 @ 0x43d2d6 0x4079ee 0x40759d 0x981c70 0x471041
#       0x981c6f        github.com/slok/goresilience/timeout.NewMiddleware.func1.1.1+0x4f       /root/go/pkg/mod/github.com/slok/goresilience@v0.2.0/timeout/timeout.go:61

711 @ 0x43d2d6 0x44d91e 0x75e0ab 0x471041
#       0x75e0aa        net/http.setRequestCancel.func4+0x8a    /usr/local/go/src/net/http/client.go:398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants