Resolve docker.NewResolver race condition

djdongjin · djdongjin · commit 31c466f82290 · 2023-07-10T12:33:49.000-07:00
Signed-off-by: Jin Dong <djdongjin95@gmail.com> (cherry picked from commit cdb153e) Signed-off-by: Jin Dong <jin.dong@databricks.com>
diff --git a/remotes/docker/resolver.go b/remotes/docker/resolver.go
@@ -145,7 +145,11 @@ func NewResolver(options ResolverOptions) remotes.Resolver {
 
 	if options.Headers == nil {
 		options.Headers = make(http.Header)
+	} else {
+		// make a copy of the headers to avoid race due to concurrent map write
+		options.Headers = options.Headers.Clone()
 	}
+
 	if _, ok := options.Headers["User-Agent"]; !ok {
 		options.Headers.Set("User-Agent", "containerd/"+version.Version)
 	}
diff --git a/remotes/docker/resolver_test.go b/remotes/docker/resolver_test.go
@@ -46,14 +46,37 @@ func TestHTTPResolver(t *testing.T) {
 		base := s.URL[7:] // strip "http://"
 		return base, options, s.Close
 	}
-
 	runBasicTest(t, "testname", s)
 }
 
 func TestHTTPSResolver(t *testing.T) {
 	runBasicTest(t, "testname", tlsServer)
 }
 
+func TestResolverOptionsRace(t *testing.T) {
+	header := http.Header{}
+	header.Set("X-Test", "test")
+
+	s := func(h http.Handler) (string, ResolverOptions, func()) {
+		s := httptest.NewServer(h)
+
+		options := ResolverOptions{
+			Headers: header,
+		}
+		base := s.URL[7:] // strip "http://"
+		return base, options, s.Close
+	}
+
+	for i := 0; i < 5; i++ {
+		t.Run(fmt.Sprintf("test ResolverOptions race %d", i), func(t *testing.T) {
+			// parallel sub tests so the race condition (if not handled) can be caught
+			// by race detector
+			t.Parallel()
+			runBasicTest(t, "testname", s)
+		})
+	}
+}
+
 func TestBasicResolver(t *testing.T) {
 	basicAuth := func(h http.Handler) (string, ResolverOptions, func()) {
 		// Wrap with basic auth

Original file line number	Diff line number	Diff line change
`@@ -145,7 +145,11 @@ func NewResolver(options ResolverOptions) remotes.Resolver {`
`145`	`145`
`146`	`146`	`if options.Headers == nil {`
`147`	`147`	`options.Headers = make(http.Header)`
	`148`	`+ } else {`
	`149`	`+ // make a copy of the headers to avoid race due to concurrent map write`
	`150`	`+ options.Headers = options.Headers.Clone()`
`148`	`151`	`}`
	`152`	`+`
`149`	`153`	`if _, ok := options.Headers["User-Agent"]; !ok {`
`150`	`154`	`options.Headers.Set("User-Agent", "containerd/"+version.Version)`
`151`	`155`	`}`