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 HTTPTransport #731

Open
kucherenkovova opened this issue Oct 9, 2023 · 5 comments
Open

goroutine leak in HTTPTransport #731

kucherenkovova opened this issue Oct 9, 2023 · 5 comments

Comments

@kucherenkovova
Copy link

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
Copy link
Member

cleptric commented Oct 9, 2023

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
Copy link
Author

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
Copy link
Member

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

@vaind
Copy link
Collaborator

vaind commented Nov 2, 2023

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: #111

@greywolve
Copy link
Contributor

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

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

No branches or pull requests

4 participants