goroutine leak in HTTPTransport

[kucherenkovova]

Summary

You iterate over a channel in a separate goroutine. This channel is never closed which leads to a goroutine leak.

Steps To Reproduce

Install "go.uber.org/goleak" and add the following code to transport_test.go file

package sentry

import (
        ...
	"go.uber.org/goleak"
)

...

func TestHTTPTransportDoesntLeakGoroutines(t *testing.T) {
	// verify that we're not leaking any goroutine
	defer goleak.VerifyNone(t)

	// init and configure transport
	transport := NewHTTPTransport()
	transport.Configure(ClientOptions{
		Dsn:        "https://test@foobar/1",
		HTTPClient: http.DefaultClient,
	})
	// use transport if needed

	// the transport is not needed anymore, get rid of it
	transport = nil

	// do some other work here
}

output:

=== RUN   TestHTTPTransportDoesntLeakGoroutines
    transport_test.go:701: found unexpected goroutines:
        [Goroutine 20 in state chan receive, with github.com/getsentry/sentry-go.(*HTTPTransport).worker on top of the stack:
        goroutine 20 [chan receive]:
        github.com/getsentry/sentry-go.(*HTTPTransport).worker(0x140002478f0)
        	/path/to/sentry-go/transport.go:471 +0xb8
        created by github.com/getsentry/sentry-go.(*HTTPTransport).Configure.func1 in goroutine 19
        	/path/to/sentry-go/transport.go:343 +0x60
        ]

Expected Behavior

As a user, I want to be able to tear down all the sentry-allocated resources when it's no longer needed.

SDK

• sentry-go version: v0.25.0
• Go version: go version go1.20.9
• Using Go Modules? yes

[cleptric]

Could you explain your use case in a bit more detail? By default, the SDK is supposed to run all the time so it can capture errors and send performance data.

[kucherenkovova]

Hi @cleptric! We don't explicitly instantiate HTTPTransport in our app. Our initialization code looks like this:

hub := sentry.NewHub(nil, sentry.NewScope())
client, _ := sentry.NewClient(opts)
hub.BindClient(client)
...
hub.CaptureException(err)

I noticed that during the shutdown process, our service has a leaked goroutine. It is not critical for my case at all. I agree that most of the use cases for sentry fall into category "live as long as the process does". However, I can think of resource-limited environments where one might want to tear down all the sentry-allocated resources when it's no longer needed. It'd be really nice to have some API for this behavior.

[cleptric]

I would recommend using the HTTPSyncTransport, see https://docs.sentry.io/platforms/go/configuration/transports/#usage.

[vaind]

I've encountered this too when working on Profiling in the past - it's causing issues in tests because we have outstanding HTTP transport workers that stay running indefinitely.

Related: https://github.com/getsentry/sentry-go/issues/111

[github-actions[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[greywolve]

I guess this can be solved in #111 which is still open and on the backlog