Browse code

Fix some data races

After running the test suite with the race detector enabled I found
these gems that need to be fixed.
This is just round one, sadly lost my test results after I built the
binary to test this... (whoops)

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2017/02/01 11:03:51
Showing 8 changed files
... ...
@@ -28,6 +28,14 @@ type EndpointIPAMConfig struct {
28 28
 	LinkLocalIPs []string `json:",omitempty"`
29 29
 }
30 30
 
31
+// Copy makes a copy of the endpoint ipam config
32
+func (cfg *EndpointIPAMConfig) Copy() *EndpointIPAMConfig {
33
+	cfgCopy := *cfg
34
+	cfgCopy.LinkLocalIPs = make([]string, 0, len(cfg.LinkLocalIPs))
35
+	cfgCopy.LinkLocalIPs = append(cfgCopy.LinkLocalIPs, cfg.LinkLocalIPs...)
36
+	return &cfgCopy
37
+}
38
+
31 39
 // PeerInfo represents one peer of an overlay network
32 40
 type PeerInfo struct {
33 41
 	Name string
... ...
@@ -52,6 +60,25 @@ type EndpointSettings struct {
52 52
 	MacAddress          string
53 53
 }
54 54
 
55
+// Copy makes a deep copy of `EndpointSettings`
56
+func (es *EndpointSettings) Copy() *EndpointSettings {
57
+	epCopy := *es
58
+	if es.IPAMConfig != nil {
59
+		epCopy.IPAMConfig = es.IPAMConfig.Copy()
60
+	}
61
+
62
+	if es.Links != nil {
63
+		links := make([]string, 0, len(es.Links))
64
+		epCopy.Links = append(links, es.Links...)
65
+	}
66
+
67
+	if es.Aliases != nil {
68
+		aliases := make([]string, 0, len(es.Aliases))
69
+		epCopy.Aliases = append(aliases, es.Aliases...)
70
+	}
71
+	return &epCopy
72
+}
73
+
55 74
 // NetworkingConfig represents the container's networking configuration for each of its interfaces
56 75
 // Carries the networking configs specified in the `docker run` and `docker network connect` commands
57 76
 type NetworkingConfig struct {
... ...
@@ -16,6 +16,7 @@ import (
16 16
 	"github.com/docker/docker/pkg/signal"
17 17
 	"github.com/docker/docker/pkg/system"
18 18
 	"github.com/docker/docker/pkg/truncindex"
19
+	"github.com/docker/docker/runconfig"
19 20
 	"github.com/docker/go-connections/nat"
20 21
 )
21 22
 
... ...
@@ -201,6 +202,7 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
201 201
 		return err
202 202
 	}
203 203
 
204
+	runconfig.SetDefaultNetModeIfBlank(hostConfig)
204 205
 	container.HostConfig = hostConfig
205 206
 	return container.ToDisk()
206 207
 }
... ...
@@ -143,7 +143,7 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) (
143 143
 	}
144 144
 	// Make sure NetworkMode has an acceptable value. We do this to ensure
145 145
 	// backwards API compatibility.
146
-	container.HostConfig = runconfig.SetDefaultNetModeIfBlank(container.HostConfig)
146
+	runconfig.SetDefaultNetModeIfBlank(container.HostConfig)
147 147
 
148 148
 	daemon.updateContainerNetworkSettings(container, endpointsConfigs)
149 149
 
... ...
@@ -11,6 +11,7 @@ import (
11 11
 	"github.com/docker/docker/api/types/versions/v1p20"
12 12
 	"github.com/docker/docker/container"
13 13
 	"github.com/docker/docker/daemon/network"
14
+	"github.com/docker/go-connections/nat"
14 15
 )
15 16
 
16 17
 // ContainerInspect returns low-level information about a
... ...
@@ -45,7 +46,8 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
45 45
 	apiNetworks := make(map[string]*networktypes.EndpointSettings)
46 46
 	for name, epConf := range container.NetworkSettings.Networks {
47 47
 		if epConf.EndpointSettings != nil {
48
-			apiNetworks[name] = epConf.EndpointSettings
48
+			// We must make a copy of this pointer object otherwise it can race with other operations
49
+			apiNetworks[name] = epConf.EndpointSettings.Copy()
49 50
 		}
50 51
 	}
51 52
 
... ...
@@ -57,7 +59,6 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
57 57
 			HairpinMode:            container.NetworkSettings.HairpinMode,
58 58
 			LinkLocalIPv6Address:   container.NetworkSettings.LinkLocalIPv6Address,
59 59
 			LinkLocalIPv6PrefixLen: container.NetworkSettings.LinkLocalIPv6PrefixLen,
60
-			Ports:                  container.NetworkSettings.Ports,
61 60
 			SandboxKey:             container.NetworkSettings.SandboxKey,
62 61
 			SecondaryIPAddresses:   container.NetworkSettings.SecondaryIPAddresses,
63 62
 			SecondaryIPv6Addresses: container.NetworkSettings.SecondaryIPv6Addresses,
... ...
@@ -66,6 +67,12 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
66 66
 		Networks:               apiNetworks,
67 67
 	}
68 68
 
69
+	ports := make(nat.PortMap, len(container.NetworkSettings.Ports))
70
+	for k, pm := range container.NetworkSettings.Ports {
71
+		ports[k] = pm
72
+	}
73
+	networkSettings.NetworkSettingsBase.Ports = ports
74
+
69 75
 	return &types.ContainerJSON{
70 76
 		ContainerJSONBase: base,
71 77
 		Mounts:            mountPoints,
... ...
@@ -31,7 +31,10 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error {
31 31
 	case libcontainerd.StateExit:
32 32
 		// if container's AutoRemove flag is set, remove it after clean up
33 33
 		autoRemove := func() {
34
-			if c.HostConfig.AutoRemove {
34
+			c.Lock()
35
+			ar := c.HostConfig.AutoRemove
36
+			c.Unlock()
37
+			if ar {
35 38
 				if err := daemon.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: true, RemoveVolume: true}); err != nil {
36 39
 					logrus.Errorf("can't remove container %s: %v", c.ID, err)
37 40
 				}
... ...
@@ -15,7 +15,6 @@ import (
15 15
 	"github.com/docker/docker/api/types"
16 16
 	containertypes "github.com/docker/docker/api/types/container"
17 17
 	"github.com/docker/docker/container"
18
-	"github.com/docker/docker/runconfig"
19 18
 )
20 19
 
21 20
 // ContainerStart starts a container.
... ...
@@ -138,10 +137,6 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint
138 138
 		return err
139 139
 	}
140 140
 
141
-	// Make sure NetworkMode has an acceptable value. We do this to ensure
142
-	// backwards API compatibility.
143
-	container.HostConfig = runconfig.SetDefaultNetModeIfBlank(container.HostConfig)
144
-
145 141
 	if err := daemon.initializeNetworking(container); err != nil {
146 142
 		return err
147 143
 	}
... ...
@@ -53,7 +53,7 @@ func (w *ContainerConfigWrapper) getHostConfig() *container.HostConfig {
53 53
 
54 54
 	// Make sure NetworkMode has an acceptable value. We do this to ensure
55 55
 	// backwards compatible API behavior.
56
-	hc = SetDefaultNetModeIfBlank(hc)
56
+	SetDefaultNetModeIfBlank(hc)
57 57
 
58 58
 	return hc
59 59
 }
... ...
@@ -25,11 +25,10 @@ func DecodeHostConfig(src io.Reader) (*container.HostConfig, error) {
25 25
 // to default if it is not populated. This ensures backwards compatibility after
26 26
 // the validation of the network mode was moved from the docker CLI to the
27 27
 // docker daemon.
28
-func SetDefaultNetModeIfBlank(hc *container.HostConfig) *container.HostConfig {
28
+func SetDefaultNetModeIfBlank(hc *container.HostConfig) {
29 29
 	if hc != nil {
30 30
 		if hc.NetworkMode == container.NetworkMode("") {
31 31
 			hc.NetworkMode = container.NetworkMode("default")
32 32
 		}
33 33
 	}
34
-	return hc
35 34
 }