Skip to content

HTTP/3: Potential race between AddConn and RoundTrip #457

Description

@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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugpriority:highData loss, memory leak, or panic in productionquic-goInvolves quic-go, HTTP/3, or internal/http3 modified code

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions