HTTP/3: Potential race between AddConn and RoundTrip

[danielboros]

I believe there's a race between the two paths when using HTTP/3 for the transport. I found this when hitting a panic here in internal/http3/transport.go:

return conn, t.newClientConn(conn), nil

I had a suspicion here because the wrapping path (transport.go) was recently refactored to use http3.Transport instead of http3.RoundTripper. This only happens intermittently but it is prevalent when using imroc/req in a highly trafficked environment that generates a bunch of custom clients.

Anyway, this t.newClientConn is initialized by:

func (t *Transport) init() error

... which normally is initialized inside roundTripOpt:

t.initOnce.Do(func() { t.initErr = t.init() })

Except this doesn't happen when calling this separate path:

// AddConn add a http3 connection, dial new conn if not exists.
func (t *Transport) AddConn(ctx context.Context, addr string) error {
	addr = authorityAddr(addr)
	cl, _, err := t.getClient(ctx, addr, false)
	if err == nil {
		cl.useCount.Add(-1)
	}
	return err
}

The whole chain to handlePendingAltSvc seems like it can potentially race after launching the Goroutine, I think. Let me know what you think.

[imroc]

Do you have any panic log? Is #456 related

[danielboros]

We're maintaining a fork but this is the path. The line numbers should be the same as v3.54.1.

goroutine 104066 [running]:
github.com/imroc/req/v3/internal/http3.(*Transport).dial(0xc011eeda40, {0x3934360, 0xc0101b8140}, {0xc0135c1ed8, 0x11})
        /go/pkg/mod/github.com/banyansecurity/req/v3@v3.0.0-20250801185154-326e9f9d3b31/internal/http3/transport.go:405 +0x21a
github.com/imroc/req/v3/internal/http3.(*Transport).getClient.func1()
        /go/pkg/mod/github.com/banyansecurity/req/v3@v3.0.0-20250801185154-326e9f9d3b31/internal/http3/transport.go:328 +0x71
created by github.com/imroc/req/v3/internal/http3.(*Transport).getClient in goroutine 104083
        /go/pkg/mod/github.com/banyansecurity/req/v3@v3.0.0-20250801185154-326e9f9d3b31/internal/http3/transport.go:325 +0x292

This change appears to have fixed it and we no longer see any intermittent panics:

banyansecurity@ff7086d

[imroc]

Thanks for the detailed report. This involves the modified internal/http3/ code (forked from quic-go), so it needs careful review — the AddConn → getClient path may bypass the initOnce initialization that roundTripOpt relies on, which could indeed cause a race when AddConn and RoundTrip are called concurrently.

The getClient → newClientConn chain in the HTTP/3 transport is modified from upstream quic-go, so the standard upstream fix (if any) would need to be ported manually. This is flagged as high priority for investigation.

[Mootjelh]

Reproduced this on master and opened #538.

AddConn is the second way into getClient and only roundTripOpt ran init, so a transport whose first use is AddConn reaches dial with a nil quic.Transport and goes down in the dial goroutine. tr := &Transport{} plus one AddConn is enough.

On the porting question: quic-go has no AddConn, so roundTripOpt is its only way into getClient and init always runs first there. There is nothing upstream to port, the second door is this repo's.

It is intermittent because t3.RoundTripOnlyCachedConn runs on every https request once HTTP/3 is on, and that runs init first most of the time.

@danielboros thanks for the report, the fix is the same initOnce.Do call you landed in your fork. I left out the ErrNilNewClientConn guard and said why in the PR.