daemon: move default stop-timeout to containerStop()

This avoids having to determine what the default is in various
parts of the code. If no custom timeout is passed (nil), the
default will be used.

Also remove the named return variable from cleanupContainer(),
as it wasn't used.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/daemon.go

@@ -1116,10 +1116,8 @@ func (daemon *Daemon) waitForStartupDone() {
 }
 
 func (daemon *Daemon) shutdownContainer(c *container.Container) error {
-	stopTimeout := c.StopTimeout()
-
 	// If container failed to exit in stopTimeout seconds of SIGTERM, then using the force
-	if err := daemon.containerStop(c, stopTimeout); err != nil {
+	if err := daemon.containerStop(c, nil); err != nil {
 		return fmt.Errorf("Failed to stop container %s with error: %v", c.ID, err)
 	}
 

daemon/delete.go

@@ -97,7 +97,19 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, config ty
 	// if stats are currently getting collected.
 	daemon.statsCollector.StopCollection(container)
 
-	if err := daemon.containerStop(container, 3); err != nil {
+	// stopTimeout is the number of seconds to wait for the container to stop
+	// gracefully before forcibly killing it.
+	//
+	// Why 3 seconds? The timeout specified here was originally added in commit
+	// 1615bb08c7c3fc6c4b22db0a633edda516f97cf0, which added a custom timeout to
+	// some commands, but lacking an option for a timeout on "docker rm", was
+	// hardcoded to 10 seconds. Commit 28fd289b448164b77affd8103c0d96fd8110daf9
+	// later on updated this to 3 seconds (but no background on that change).
+	//
+	// If you arrived here and know the answer, you earned yourself a picture
+	// of a cute animal of your own choosing.
+	var stopTimeout = 3
+	if err := daemon.containerStop(container, &stopTimeout); err != nil {
 		return err
 	}
 

daemon/restart.go

@@ -19,11 +19,8 @@ func (daemon *Daemon) ContainerRestart(name string, seconds *int) error {
 	if err != nil {
 		return err
 	}
-	if seconds == nil {
+	err = daemon.containerRestart(ctr, seconds)
-		stopTimeout := ctr.StopTimeout()
+	if err != nil {
-		seconds = &stopTimeout
-	}
-	if err := daemon.containerRestart(ctr, *seconds); err != nil {
 		return fmt.Errorf("Cannot restart container %s: %v", name, err)
 	}
 	return nil
@@ -34,8 +31,7 @@ func (daemon *Daemon) ContainerRestart(name string, seconds *int) error {
 // container. When stopping, wait for the given duration in seconds to
 // gracefully stop, before forcefully terminating the container. If
 // given a negative duration, wait forever for a graceful stop.
-func (daemon *Daemon) containerRestart(container *container.Container, seconds int) error {
+func (daemon *Daemon) containerRestart(container *container.Container, seconds *int) error {
-
 	// Determine isolation. If not specified in the hostconfig, use daemon default.
 	actualIsolation := container.HostConfig.Isolation
 	if containertypes.Isolation.IsDefault(actualIsolation) {

daemon/stop.go

@@ -24,34 +24,39 @@ func (daemon *Daemon) ContainerStop(name string, timeout *int) error {
 		return err
 	}
 	if !ctr.IsRunning() {
-		return containerNotModifiedError{running: false}
+		return containerNotModifiedError{}
 	}
-	if timeout == nil {
+	err = daemon.containerStop(ctr, timeout)
-		stopTimeout := ctr.StopTimeout()
+	if err != nil {
-		timeout = &stopTimeout
-	}
-	if err := daemon.containerStop(ctr, *timeout); err != nil {
 		return errdefs.System(errors.Wrapf(err, "cannot stop container: %s", name))
 	}
 	return nil
 }
 
 // containerStop sends a stop signal, waits, sends a kill signal.
-func (daemon *Daemon) containerStop(ctr *container.Container, seconds int) error {
+func (daemon *Daemon) containerStop(ctr *container.Container, seconds *int) error {
 	// TODO propagate a context down to this function
 	ctx := context.TODO()
 	if !ctr.IsRunning() {
 		return nil
 	}
+
+	var (
+		stopSignal  = ctr.StopSignal()
+		stopTimeout = ctr.StopTimeout()
+	)
+	if seconds != nil {
+		stopTimeout = *seconds
+	}
+
 	var wait time.Duration
-	if seconds >= 0 {
+	if stopTimeout >= 0 {
-		wait = time.Duration(seconds) * time.Second
+		wait = time.Duration(stopTimeout) * time.Second
 	}
 	success := func() error {
 		daemon.LogContainerEvent(ctr, "stop")
 		return nil
 	}
-	stopSignal := ctr.StopSignal()
 
 	// 1. Send a stop signal
 	err := daemon.killPossiblyDeadProcess(ctr, stopSignal)
@@ -61,7 +66,7 @@ func (daemon *Daemon) containerStop(ctr *container.Container, seconds int) error
 
 	var subCtx context.Context
 	var cancel context.CancelFunc
-	if seconds >= 0 {
+	if stopTimeout >= 0 {
 		subCtx, cancel = context.WithTimeout(ctx, wait)
 	} else {
 		subCtx, cancel = context.WithCancel(ctx)
@@ -77,7 +82,7 @@ func (daemon *Daemon) containerStop(ctr *container.Container, seconds int) error
 		// the container has still not exited, and the kill function errored, so log the error here:
 		logrus.WithError(err).WithField("container", ctr.ID).Errorf("Error sending stop (signal %d) to container", stopSignal)
 	}
-	if seconds < 0 {
+	if stopTimeout < 0 {
 		// if the client requested that we never kill / wait forever, but container.Wait was still
 		// interrupted (parent context cancelled, for example), we should propagate the signal failure
 		return err