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

Onetimepool Goroutine Leaks #506

Closed
osxtest opened this issue Aug 31, 2024 · 1 comment · Fixed by #507
Closed

Onetimepool Goroutine Leaks #506

osxtest opened this issue Aug 31, 2024 · 1 comment · Fixed by #507
Labels
Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@osxtest
Copy link
Contributor

osxtest commented Aug 31, 2024

Description:
I found an issue while using the tlsx package, but the root cause seems to be in the utils package. Therefore, I am submitting the issue here.

Environment:

  • Go version: go1.21
  • System: macOS

Steps to Reproduce:

  1. Run the test code in here:
package main

import (
	"crypto/tls"
	"log"
	"net/http"
	"net/http/httptest"
	_ "net/http/pprof"
	"net/url"
	"testing"

	"github.com/projectdiscovery/tlsx/pkg/tlsx"
	"github.com/projectdiscovery/tlsx/pkg/tlsx/clients"
)

func TestGoroutineLeak(t *testing.T) {
	go func() {
		log.Println(http.ListenAndServe("localhost:6060", nil))
	}()
	defer func() {
		t.Log("http://localhost:6060/debug/pprof/")
		select {}
	}()

	ts := httptest.NewUnstartedServer(nil)
	ts.TLS = &tls.Config{
		CipherSuites: []uint16{tls.TLS_RSA_WITH_RC4_128_SHA},
		MinVersion:   tls.VersionTLS10,
		MaxVersion:   tls.VersionTLS13,
	}
	ts.StartTLS()
	defer ts.Close()

	opts := &clients.Options{
		Retries:         1,
		TlsVersionsEnum: true,
		TlsCiphersEnum:  true,
	}
	service, err := tlsx.New(opts)
	if err != nil {
		panic(err)
	}
	parsed, _ := url.Parse(ts.URL)
	_, err = service.Connect(parsed.Hostname(), parsed.Hostname(), parsed.Port())
	if err != nil {
		panic(err)
	}
}
  1. After running the test, visit http://localhost:6060/debug/pprof/goroutine?debug=1.
  2. You should see a onetimepool goroutine leak.

Observed Output:

3 @ 0x10003daae 0x100008ef0 0x100008b37 0x100a7a9ee 0x100a7fc4f 0x100072b61
#	0x100a7a9ed	github.com/projectdiscovery/utils/conn/connpool.(*OneTimePool).Run+0x2ad		/Users/qwerty/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/projectdiscovery/utils@v0.2.5/conn/connpool/onetimepool.go:78
#	0x100a7fc4e	github.com/projectdiscovery/tlsx/pkg/tlsx/tls.(*Client).EnumerateCiphers.func1+0x2e	/Users/qwerty/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/projectdiscovery/tlsx@v1.1.7/pkg/tlsx/tls/tls.go:218

This trace points to a potential goroutine leak in here

p.idleConnections <- conn

Expected Behavior:
No goroutine leaks should be present.

Analysis:
The leak reason is that after pool.Acquire but before pool.Close, if the p.idleConnections channel is full, pool.Run gets stuck at sending a connection to the channel. Hence, after pool.Close, it doesn't get the chance to go to case <-p.ctx.Done(): and return.

@Mzack9999 Mzack9999 added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Sep 2, 2024
@Mzack9999
Copy link
Member

@osxtest Nice catch! Thanks for providing a fix!

@Mzack9999 Mzack9999 added the Status: Completed Nothing further to be done with this issue. Awaiting to be closed. label Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants