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

[greywolve]

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