go routine leaks during clone

[joshjennings98]

We have run into an occasionally problem where we use git clone to fetch a repository but then later on when trying to upload the cloned repository to one of our services, we get a precondition error.

We suspected that we weren't closing our go routines and as part of the debugging we used goleak to verify whether we had any open go routines. This showed us that we did have some leaky go routines. Further investigation led us to testing the PlainClone function for leaks and we discovered that it has a leak.

This being the issue could explain the problem we are having in our services as it suggests that the repository or it's metadata might not completely cloned when we progress in our code.

To reproduce take one of the examples and add the goleak check.

func TestPlainClone(t *testing.T) {
	defer goleak.VerifyNone(t)
	_, err := PlainClone("test-dir", false, &CloneOptions{
		URL: "https://github.com/go-git/go-git",
	}, t)
	require.NoError(t, err)
}

This resulted in the following error:

=== RUN   TestPlainClone
    /home/josh/go-git/test.go:18: found unexpected goroutines:
        [Goroutine 50 in state IO wait, with internal/poll.runtime_pollWait on top of the stack:
        goroutine 50 [IO wait]:
        internal/poll.runtime_pollWait(0x7f7c63778628, 0x72)
                /home/josh/go/src/runtime/netpoll.go:305 +0x89
        internal/poll.(*pollDesc).wait(0xc000142e00?, 0xc0004b1300?, 0x0)
                /home/josh/go/src/internal/poll/fd_poll_runtime.go:84 +0x32
        internal/poll.(*pollDesc).waitRead(...)
                /home/josh/go/src/internal/poll/fd_poll_runtime.go:89
        internal/poll.(*FD).Read(0xc000142e00, {0xc0004b1300, 0x1300, 0x1300})
                /home/josh/go/src/internal/poll/fd_unix.go:167 +0x25a
        net.(*netFD).Read(0xc000142e00, {0xc0004b1300?, 0x7e5501?, 0xc0001ffc80?})
                /home/josh/go/src/net/fd_posix.go:55 +0x29
        net.(*conn).Read(0xc0000b2010, {0xc0004b1300?, 0xd90?, 0xc0001ffc80?})
                /home/josh/go/src/net/net.go:183 +0x45
        crypto/tls.(*atLeastReader).Read(0xc00034e0d8, {0xc0004b1300?, 0x0?, 0x4e0f28?})
                /home/josh/go/src/crypto/tls/conn.go:787 +0x3d
        bytes.(*Buffer).ReadFrom(0xc0000c2278, {0x8d7f20, 0xc00034e0d8})
                /home/josh/go/src/bytes/buffer.go:202 +0x98
        crypto/tls.(*Conn).readFromUntil(0xc0000c2000, {0x8d8760?, 0xc0000b2010}, 0xc0004b1875?)
                /home/josh/go/src/crypto/tls/conn.go:809 +0xe5
        crypto/tls.(*Conn).readRecordOrCCS(0xc0000c2000, 0x0)
                /home/josh/go/src/crypto/tls/conn.go:616 +0x116
        crypto/tls.(*Conn).readRecord(...)
                /home/josh/go/src/crypto/tls/conn.go:582
        crypto/tls.(*Conn).Read(0xc0000c2000, {0xc00019c000, 0x1000, 0x476029?})
                /home/josh/go/src/crypto/tls/conn.go:1287 +0x16f
        bufio.(*Reader).Read(0xc00068f440, {0xc0002d89e0, 0x9, 0x6db2c5?})
                /home/josh/go/src/bufio/bufio.go:237 +0x1bb
        io.ReadAtLeast({0x8d7da0, 0xc00068f440}, {0xc0002d89e0, 0x9, 0x9}, 0x9)
                /home/josh/go/src/io/io.go:332 +0x9a
        io.ReadFull(...)
                /home/josh/go/src/io/io.go:351
        net/http.http2readFrameHeader({0xc0002d89e0?, 0x9?, 0xc00007e960?}, {0x8d7da0?, 0xc00068f440?})
                /home/josh/go/src/net/http/h2_bundle.go:1565 +0x6e
        net/http.(*http2Framer).ReadFrame(0xc0002d89a0)
                /home/josh/go/src/net/http/h2_bundle.go:1829 +0x95
        net/http.(*http2clientConnReadLoop).run(0xc00061df98)
                /home/josh/go/src/net/http/h2_bundle.go:8875 +0x130
        net/http.(*http2ClientConn).readLoop(0xc0000c0180)
                /home/josh/go/src/net/http/h2_bundle.go:8771 +0x6f
        created by net/http.(*http2Transport).newClientConn
                /home/josh/go/src/net/http/h2_bundle.go:7478 +0xaaa
        ]
--- FAIL: TestPlainClone (1.84s)
FAIL
FAIL    github.com/go-git/go-git/v5     1.841s

The version of golang we are using is go version go1.19.2 linux/amd64.

I did a brief search of the issues but could not find anything relevant to this problem.

Thanks for any help you can provide.

[acabarbaye]

This seems to be related to this other concurrency issue #629

[pjbgf]

This is not related to #629 and it is also not a leak.

The behaviour observed is related to the Go Transport as TCP Keep Alive is enabled by default (for performance reasons).
If you register a transport with keep alive disabled you will notice that no new go routines will outlive the test:

	noKeepAlive := http.DefaultTransport.(*http.Transport).Clone()
	noKeepAlive.DisableKeepAlives = true
	client.InstallProtocol("https", githttp.NewClient(&http.Client{Transport: noKeepAlive}))