From 500f559311a019cfc63ca04afb5607c54b2edc22 Mon Sep 17 00:00:00 2001 From: jchadwick-buf <116005195+jchadwick-buf@users.noreply.github.com> Date: Thu, 6 Jul 2023 13:29:02 -0400 Subject: [PATCH] Clarify HTTP/2 data race issue (#47) --- Makefile | 1 + go.mod | 2 +- transport.go | 10 ++++++---- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 1502e39..09294d4 100644 --- a/Makefile +++ b/Makefile @@ -36,6 +36,7 @@ build: generate ## Build all packages .PHONY: generate generate: $(BIN)/license-header ## Regenerate code and licenses + go mod tidy @# We want to operate on a list of modified and new files, excluding @# deleted and ignored files. git-ls-files can't do this alone. comm -23 takes @# two files and prints the union, dropping lines common to both (-3) and diff --git a/go.mod b/go.mod index 1ce0723..a723ac0 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/bufbuild/httplb go 1.19 require ( + github.com/jonboulle/clockwork v0.4.0 github.com/stretchr/testify v1.8.4 golang.org/x/net v0.10.0 golang.org/x/sync v0.2.0 @@ -10,7 +11,6 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/jonboulle/clockwork v0.4.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/text v0.9.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/transport.go b/transport.go index 0ff95c4..9dcb8de 100644 --- a/transport.go +++ b/transport.go @@ -418,10 +418,12 @@ func (t *transportPool) NewConn(address resolver.Address) (conn.Conn, bool) { return nil, false } - // NOTE: The Go HTTP2 client does NOT defensively clone the TLS config - // before it attempts to mutate it, so we can't share the same TLS config - // across multiple connections right now. - // TODO: file upstream bug? + // NOTE: When using ForceAttemptHTTP2, Go can mutate the TLSClientConfig + // without first making a defensive copy. This is intended, though not + // documented. + // https://github.com/golang/go/issues/14275 + // TODO: Possibly move to factory, since that's where ForceAttemptHTTP2 is + // actually set? opts := t.roundTripperOptions opts.TLSClientConfig = opts.TLSClientConfig.Clone()