Merge pull request #30618 from cpuguy83/fix_data_races

Fix some data races

api/types/network/network.go

@@ -28,6 +28,14 @@ type EndpointIPAMConfig struct {
 	LinkLocalIPs []string `json:",omitempty"`
 }
 
+// Copy makes a copy of the endpoint ipam config
+func (cfg *EndpointIPAMConfig) Copy() *EndpointIPAMConfig {
+	cfgCopy := *cfg
+	cfgCopy.LinkLocalIPs = make([]string, 0, len(cfg.LinkLocalIPs))
+	cfgCopy.LinkLocalIPs = append(cfgCopy.LinkLocalIPs, cfg.LinkLocalIPs...)
+	return &cfgCopy
+}
+
 // PeerInfo represents one peer of an overlay network
 type PeerInfo struct {
 	Name string
@@ -52,6 +60,25 @@ type EndpointSettings struct {
 	MacAddress          string
 }
 
+// Copy makes a deep copy of `EndpointSettings`
+func (es *EndpointSettings) Copy() *EndpointSettings {
+	epCopy := *es
+	if es.IPAMConfig != nil {
+		epCopy.IPAMConfig = es.IPAMConfig.Copy()
+	}
+
+	if es.Links != nil {
+		links := make([]string, 0, len(es.Links))
+		epCopy.Links = append(links, es.Links...)
+	}
+
+	if es.Aliases != nil {
+		aliases := make([]string, 0, len(es.Aliases))
+		epCopy.Aliases = append(aliases, es.Aliases...)
+	}
+	return &epCopy
+}
+
 // NetworkingConfig represents the container's networking configuration for each of its interfaces
 // Carries the networking configs specified in the `docker run` and `docker network connect` commands
 type NetworkingConfig struct {

daemon/container.go

@@ -16,6 +16,7 @@ import (
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/truncindex"
+	"github.com/docker/docker/runconfig"
 	"github.com/docker/go-connections/nat"
 )
 
@@ -199,6 +200,7 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
 		return err
 	}
 
+	runconfig.SetDefaultNetModeIfBlank(hostConfig)
 	container.HostConfig = hostConfig
 	return container.ToDisk()
 }

daemon/create.go

@@ -143,7 +143,7 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) (
 	}
 	// Make sure NetworkMode has an acceptable value. We do this to ensure
 	// backwards API compatibility.
-	container.HostConfig = runconfig.SetDefaultNetModeIfBlank(container.HostConfig)
+	runconfig.SetDefaultNetModeIfBlank(container.HostConfig)
 
 	daemon.updateContainerNetworkSettings(container, endpointsConfigs)
 

daemon/inspect.go

@@ -11,6 +11,7 @@ import (
 	"github.com/docker/docker/api/types/versions/v1p20"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/daemon/network"
+	"github.com/docker/go-connections/nat"
 )
 
 // ContainerInspect returns low-level information about a
@@ -45,7 +46,8 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
 	apiNetworks := make(map[string]*networktypes.EndpointSettings)
 	for name, epConf := range container.NetworkSettings.Networks {
 		if epConf.EndpointSettings != nil {
-			apiNetworks[name] = epConf.EndpointSettings
+			// We must make a copy of this pointer object otherwise it can race with other operations
+			apiNetworks[name] = epConf.EndpointSettings.Copy()
 		}
 	}
 
@@ -57,7 +59,6 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
 			HairpinMode:            container.NetworkSettings.HairpinMode,
 			LinkLocalIPv6Address:   container.NetworkSettings.LinkLocalIPv6Address,
 			LinkLocalIPv6PrefixLen: container.NetworkSettings.LinkLocalIPv6PrefixLen,
-			Ports:                  container.NetworkSettings.Ports,
 			SandboxKey:             container.NetworkSettings.SandboxKey,
 			SecondaryIPAddresses:   container.NetworkSettings.SecondaryIPAddresses,
 			SecondaryIPv6Addresses: container.NetworkSettings.SecondaryIPv6Addresses,
@@ -66,6 +67,12 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
 		Networks:               apiNetworks,
 	}
 
+	ports := make(nat.PortMap, len(container.NetworkSettings.Ports))
+	for k, pm := range container.NetworkSettings.Ports {
+		ports[k] = pm
+	}
+	networkSettings.NetworkSettingsBase.Ports = ports
+
 	return &types.ContainerJSON{
 		ContainerJSONBase: base,
 		Mounts:            mountPoints,

daemon/monitor.go

@@ -31,7 +31,10 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error {
 	case libcontainerd.StateExit:
 		// if container's AutoRemove flag is set, remove it after clean up
 		autoRemove := func() {
-			if c.HostConfig.AutoRemove {
+			c.Lock()
+			ar := c.HostConfig.AutoRemove
+			c.Unlock()
+			if ar {
 				if err := daemon.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: true, RemoveVolume: true}); err != nil {
 					logrus.Errorf("can't remove container %s: %v", c.ID, err)
 				}

daemon/start.go

@@ -15,7 +15,6 @@ import (
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
-	"github.com/docker/docker/runconfig"
 )
 
 // ContainerStart starts a container.
@@ -138,10 +137,6 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint
 		return err
 	}
 
-	// Make sure NetworkMode has an acceptable value. We do this to ensure
-	// backwards API compatibility.
-	container.HostConfig = runconfig.SetDefaultNetModeIfBlank(container.HostConfig)
-
 	if err := daemon.initializeNetworking(container); err != nil {
 		return err
 	}

runconfig/config_unix.go

@@ -53,7 +53,7 @@ func (w *ContainerConfigWrapper) getHostConfig() *container.HostConfig {
 
 	// Make sure NetworkMode has an acceptable value. We do this to ensure
 	// backwards compatible API behavior.
-	hc = SetDefaultNetModeIfBlank(hc)
+	SetDefaultNetModeIfBlank(hc)
 
 	return hc
 }

runconfig/hostconfig.go

@@ -25,11 +25,10 @@ func DecodeHostConfig(src io.Reader) (*container.HostConfig, error) {
 // to default if it is not populated. This ensures backwards compatibility after
 // the validation of the network mode was moved from the docker CLI to the
 // docker daemon.
-func SetDefaultNetModeIfBlank(hc *container.HostConfig) *container.HostConfig {
+func SetDefaultNetModeIfBlank(hc *container.HostConfig) {
 	if hc != nil {
 		if hc.NetworkMode == container.NetworkMode("") {
 			hc.NetworkMode = container.NetworkMode("default")
 		}
 	}
-	return hc
 }