testing: fix race condition reading data from containers

cezarsa · cezarsa · commit 26cad7b36a97 · 2018-01-31T17:11:46.000-02:00
Example of a race caught by the race detector:

```
==================
WARNING: DATA RACE
Write at 0x00c4201dea68 by goroutine 374:
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).stopContainer()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:726 +0x15a
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).(github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.stopContainer)-fm()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:169 +0x5f
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).handlerWrapper.func1()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:379 +0x70a
  net/http.HandlerFunc.ServeHTTP()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:1918 +0x51
  github.com/tsuru/tsuru/vendor/github.com/gorilla/mux.(*Router).ServeHTTP()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/gorilla/mux/mux.go:133 +0x18a
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).ServeHTTP()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:340 +0x271
  net/http.serverHandler.ServeHTTP()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:2619 +0xbc
  net/http.(*conn).serve()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:1801 +0x83b
Previous read at 0x00c4201dea68 by goroutine 357:
  reflect.Value.Bool()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/reflect/value.go:248 +0x52
  encoding/json.isEmptyValue()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:309 +0xd4
  encoding/json.(*structEncoder).encode()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:626 +0x31f
  encoding/json.(*structEncoder).(encoding/json.encode)-fm()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:651 +0x7b
  encoding/json.(*structEncoder).encode()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:637 +0x2e2
  encoding/json.(*structEncoder).(encoding/json.encode)-fm()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:651 +0x7b
  encoding/json.(*ptrEncoder).encode()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:778 +0x12a
  encoding/json.(*ptrEncoder).(encoding/json.encode)-fm()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:783 +0x7b
  encoding/json.(*encodeState).reflectValue()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:323 +0x93
  encoding/json.(*encodeState).marshal()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/encode.go:296 +0xbc
  encoding/json.(*Encoder).Encode()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/encoding/json/stream.go:193 +0xda
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).inspectContainer()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:569 +0x289
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).(github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.inspectContainer)-fm()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:165 +0x5f
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).handlerWrapper.func1()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:379 +0x70a
  net/http.HandlerFunc.ServeHTTP()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:1918 +0x51
  github.com/tsuru/tsuru/vendor/github.com/gorilla/mux.(*Router).ServeHTTP()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/gorilla/mux/mux.go:133 +0x18a
  github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing.(*DockerServer).ServeHTTP()
      /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/fsouza/go-dockerclient/testing/server.go:340 +0x271
  net/http.serverHandler.ServeHTTP()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:2619 +0xbc
  net/http.(*conn).serve()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:1801 +0x83b
Goroutine 374 (running) created at:
  net/http.(*Server).Serve()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:2720 +0x37c
  net/http.Serve()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:2323 +0xe2
Goroutine 357 (finished) created at:
  net/http.(*Server).Serve()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:2720 +0x37c
  net/http.Serve()
      /home/travis/.gimme/versions/go1.9.2.linux.amd64/src/net/http/server.go:2323 +0xe2
==================
```
diff --git a/testing/server.go b/testing/server.go
@@ -547,10 +547,10 @@ func (s *DockerServer) renameContainer(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusNotFound)
 		return
 	}
-	copy := *container
-	copy.Name = r.URL.Query().Get("name")
 	s.cMut.Lock()
 	defer s.cMut.Unlock()
+	copy := *container
+	copy.Name = r.URL.Query().Get("name")
 	if s.containers[index].ID == copy.ID {
 		s.containers[index] = &copy
 	}
@@ -566,6 +566,8 @@ func (s *DockerServer) inspectContainer(w http.ResponseWriter, r *http.Request)
 	}
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
+	s.cMut.RLock()
+	defer s.cMut.RUnlock()
 	json.NewEncoder(w).Encode(container)
 }
 
@@ -820,16 +822,18 @@ func (s *DockerServer) waitContainer(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusNotFound)
 		return
 	}
+	var exitCode int
 	for {
 		time.Sleep(1e6)
 		s.cMut.RLock()
 		if !container.State.Running {
+			exitCode = container.State.ExitCode
 			s.cMut.RUnlock()
 			break
 		}
 		s.cMut.RUnlock()
 	}
-	result := map[string]int{"StatusCode": container.State.ExitCode}
+	result := map[string]int{"StatusCode": exitCode}
 	json.NewEncoder(w).Encode(result)
 }
 
@@ -1153,7 +1157,9 @@ func (s *DockerServer) createExecContainer(w http.ResponseWriter, r *http.Reques
 	}
 
 	execID := s.generateID()
+	s.cMut.Lock()
 	container.ExecIDs = append(container.ExecIDs, execID)
+	s.cMut.Unlock()
 
 	exec := docker.ExecInspect{
 		ID:          execID,

Original file line number	Diff line number	Diff line change
`@@ -547,10 +547,10 @@ func (s DockerServer) renameContainer(w http.ResponseWriter, r http.Request) {`
`547`	`547`	`http.Error(w, err.Error(), http.StatusNotFound)`
`548`	`548`	`return`
`549`	`549`	`}`
`550`		`- copy := *container`
`551`		`- copy.Name = r.URL.Query().Get("name")`
`552`	`550`	`s.cMut.Lock()`
`553`	`551`	`defer s.cMut.Unlock()`
	`552`	`+ copy := *container`
	`553`	`+ copy.Name = r.URL.Query().Get("name")`
`554`	`554`	`if s.containers[index].ID == copy.ID {`
`555`	`555`	`s.containers[index] = &copy`
`556`	`556`	`}`
`@@ -566,6 +566,8 @@ func (s DockerServer) inspectContainer(w http.ResponseWriter, r http.Request)`
`566`	`566`	`}`
`567`	`567`	`w.Header().Set("Content-Type", "application/json")`
`568`	`568`	`w.WriteHeader(http.StatusOK)`
	`569`	`+ s.cMut.RLock()`
	`570`	`+ defer s.cMut.RUnlock()`
`569`	`571`	`json.NewEncoder(w).Encode(container)`
`570`	`572`	`}`
`571`	`573`
`@@ -820,16 +822,18 @@ func (s DockerServer) waitContainer(w http.ResponseWriter, r http.Request) {`
`820`	`822`	`http.Error(w, err.Error(), http.StatusNotFound)`
`821`	`823`	`return`
`822`	`824`	`}`
	`825`	`+ var exitCode int`
`823`	`826`	`for {`
`824`	`827`	`time.Sleep(1e6)`
`825`	`828`	`s.cMut.RLock()`
`826`	`829`	`if !container.State.Running {`
	`830`	`+ exitCode = container.State.ExitCode`
`827`	`831`	`s.cMut.RUnlock()`
`828`	`832`	`break`
`829`	`833`	`}`
`830`	`834`	`s.cMut.RUnlock()`
`831`	`835`	`}`
`832`		`- result := map[string]int{"StatusCode": container.State.ExitCode}`
	`836`	`+ result := map[string]int{"StatusCode": exitCode}`
`833`	`837`	`json.NewEncoder(w).Encode(result)`
`834`	`838`	`}`
`835`	`839`
`@@ -1153,7 +1157,9 @@ func (s DockerServer) createExecContainer(w http.ResponseWriter, r http.Reques`
`1153`	`1157`	`}`
`1154`	`1158`
`1155`	`1159`	`execID := s.generateID()`
	`1160`	`+ s.cMut.Lock()`
`1156`	`1161`	`container.ExecIDs = append(container.ExecIDs, execID)`
	`1162`	`+ s.cMut.Unlock()`
`1157`	`1163`
`1158`	`1164`	`exec := docker.ExecInspect{`
`1159`	`1165`	`ID: execID,`