Merge pull request #19339 from cpuguy83/19335_revert_18736

Revert "Break big lock into some tiny locks"

container/monitor.go

@@ -80,7 +80,6 @@ type containerMonitor struct {
 // StartMonitor initializes a containerMonitor for this container with the provided supervisor and restart policy
 // and starts the container's process.
 func (container *Container) StartMonitor(s supervisor, policy container.RestartPolicy) error {
-	container.Lock()
 	container.monitor = &containerMonitor{
 		supervisor:    s,
 		container:     container,
@@ -89,7 +88,6 @@ func (container *Container) StartMonitor(s supervisor, policy container.RestartP
 		stopChan:      make(chan struct{}),
 		startSignal:   make(chan struct{}),
 	}
-	container.Unlock()
 
 	return container.monitor.wait()
 }
@@ -159,8 +157,6 @@ func (m *containerMonitor) start() error {
 		}
 		m.Close()
 	}()
-
-	m.container.Lock()
 	// reset stopped flag
 	if m.container.HasBeenManuallyStopped {
 		m.container.HasBeenManuallyStopped = false
@@ -175,20 +171,16 @@ func (m *containerMonitor) start() error {
 		if err := m.supervisor.StartLogging(m.container); err != nil {
 			m.resetContainer(false)
 
-			m.container.Unlock()
 			return err
 		}
 
 		pipes := execdriver.NewPipes(m.container.Stdin(), m.container.Stdout(), m.container.Stderr(), m.container.Config.OpenStdin)
-		m.container.Unlock()
 
 		m.logEvent("start")
 
 		m.lastStartTime = time.Now()
 
-		// don't lock Run because m.callback has own lock
 		if exitStatus, err = m.supervisor.Run(m.container, pipes, m.callback); err != nil {
-			m.container.Lock()
 			// if we receive an internal error from the initial start of a container then lets
 			// return it instead of entering the restart loop
 			// set to 127 for container cmd not found/does not exist)
@@ -198,7 +190,6 @@ func (m *containerMonitor) start() error {
 				if m.container.RestartCount == 0 {
 					m.container.ExitCode = 127
 					m.resetContainer(false)
-					m.container.Unlock()
 					return derr.ErrorCodeCmdNotFound
 				}
 			}
@@ -207,7 +198,6 @@ func (m *containerMonitor) start() error {
 				if m.container.RestartCount == 0 {
 					m.container.ExitCode = 126
 					m.resetContainer(false)
-					m.container.Unlock()
 					return derr.ErrorCodeCmdCouldNotBeInvoked
 				}
 			}
@@ -216,13 +206,11 @@ func (m *containerMonitor) start() error {
 				m.container.ExitCode = -1
 				m.resetContainer(false)
 
-				m.container.Unlock()
 				return derr.ErrorCodeCantStart.WithArgs(m.container.ID, utils.GetErrorMessage(err))
 			}
 
-			m.container.Unlock()
 			logrus.Errorf("Error running container: %s", err)
-		} // end if
+		}
 
 		// here container.Lock is already lost
 		afterRun = true
@@ -243,14 +231,13 @@ func (m *containerMonitor) start() error {
 			if m.shouldStop {
 				return err
 			}
-			m.container.Lock()
 			continue
 		}
 
 		m.logEvent("die")
 		m.resetContainer(true)
 		return err
-	} // end for
+	}
 }
 
 // resetMonitor resets the stateful fields on the containerMonitor based on the
@@ -331,7 +318,7 @@ func (m *containerMonitor) callback(processConfig *execdriver.ProcessConfig, pid
 		}
 	}
 
-	m.container.SetRunningLocking(pid)
+	m.container.SetRunning(pid)
 
 	// signal that the process has started
 	// close channel only if not closed

container/state.go

@@ -179,13 +179,6 @@ func (s *State) getExitCode() int {
 	return res
 }
 
-// SetRunningLocking locks container and sets it to "running"
-func (s *State) SetRunningLocking(pid int) {
-	s.Lock()
-	s.SetRunning(pid)
-	s.Unlock()
-}
-
 // SetRunning sets the state of the container to "running".
 func (s *State) SetRunning(pid int) {
 	s.Error = ""
@@ -199,7 +192,7 @@ func (s *State) SetRunning(pid int) {
 	s.waitChan = make(chan struct{})
 }
 
-// SetStoppedLocking locks the container state and sets it to "stopped".
+// SetStoppedLocking locks the container state is sets it to "stopped".
 func (s *State) SetStoppedLocking(exitStatus *execdriver.ExitStatus) {
 	s.Lock()
 	s.SetStopped(exitStatus)

daemon/start.go

@@ -142,15 +142,9 @@ func (daemon *Daemon) containerStart(container *container.Container) (err error)
 	mounts = append(mounts, container.TmpfsMounts()...)
 
 	container.Command.Mounts = mounts
-	container.Unlock()
-
-	// don't lock waitForStart because it has potential risk of blocking
-	// which will lead to dead lock, forever.
 	if err := daemon.waitForStart(container); err != nil {
-		container.Lock()
 		return err
 	}
-	container.Lock()
 	container.HasBeenStartedBefore = true
 	return nil
 }