Windows: [TP4] docker kill handling

Signed-off-by: John Howard <jhoward@microsoft.com>

daemon/execdriver/windows/exec.go

@@ -18,6 +18,7 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo
 		term     execdriver.Terminal
 		err      error
 		exitCode int32
+		errno    uint32
 	)
 
 	active := d.activeContainers[c.ID]
@@ -77,12 +78,15 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo
 		hooks.Start(&c.ProcessConfig, int(pid), chOOM)
 	}
 
-	if exitCode, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid); err != nil {
-		logrus.Errorf("Failed to WaitForProcessInComputeSystem %s", err)
+	if exitCode, errno, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite); err != nil {
+		if errno == hcsshim.Win32PipeHasBeenEnded {
+			logrus.Debugf("Exiting Run() after WaitForProcessInComputeSystem failed with recognised error 0x%X", errno)
+			return hcsshim.WaitErrExecFailed, nil
+		}
+		logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): 0x%X %s", errno, err)
 		return -1, err
 	}
 
-	// TODO Windows - Do something with this exit code
-	logrus.Debugln("Exiting Run() with ExitCode 0", c.ID)
+	logrus.Debugln("Exiting Run()", c.ID)
 	return int(exitCode), nil
 }

daemon/execdriver/windows/run.go

@@ -220,22 +220,19 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd
 	}
 	defer func() {
 		// Stop the container
-
-		if terminateMode {
-			logrus.Debugf("Terminating container %s", c.ID)
-			if err := hcsshim.TerminateComputeSystem(c.ID); err != nil {
-				// IMPORTANT: Don't fail if fails to change state. It could already
-				// have been stopped through kill().
-				// Otherwise, the docker daemon will hang in job wait()
-				logrus.Warnf("Ignoring error from TerminateComputeSystem %s", err)
+		if forceKill {
+			logrus.Debugf("Forcibly terminating container %s", c.ID)
+			if errno, err := hcsshim.TerminateComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil {
+				logrus.Warnf("Ignoring error from TerminateComputeSystem 0x%X %s", errno, err)
 			}
 		} else {
 			logrus.Debugf("Shutting down container %s", c.ID)
-			if err := hcsshim.ShutdownComputeSystem(c.ID); err != nil {
-				// IMPORTANT: Don't fail if fails to change state. It could already
-				// have been stopped through kill().
-				// Otherwise, the docker daemon will hang in job wait()
-				logrus.Warnf("Ignoring error from ShutdownComputeSystem %s", err)
+			if errno, err := hcsshim.ShutdownComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil {
+				if errno != hcsshim.Win32SystemShutdownIsInProgress &&
+					errno != hcsshim.Win32SpecifiedPathInvalid &&
+					errno != hcsshim.Win32SystemCannotFindThePathSpecified {
+					logrus.Warnf("Ignoring error from ShutdownComputeSystem 0x%X %s", errno, err)
+				}
 			}
 		}
 	}()
@@ -303,11 +300,20 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd
 		hooks.Start(&c.ProcessConfig, int(pid), chOOM)
 	}
 
-	var exitCode int32
-	exitCode, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid)
+	var (
+		exitCode int32
+		errno    uint32
+	)
+	exitCode, errno, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite)
 	if err != nil {
-		logrus.Errorf("Failed to WaitForProcessInComputeSystem %s", err)
-		return execdriver.ExitStatus{ExitCode: -1}, err
+		if errno != hcsshim.Win32PipeHasBeenEnded {
+			logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): %s", err)
+		}
+		// Do NOT return err here as the container would have
+		// started, otherwise docker will deadlock. It's perfectly legitimate
+		// for WaitForProcessInComputeSystem to fail in situations such
+		// as the container being killed on another thread.
+		return execdriver.ExitStatus{ExitCode: hcsshim.WaitErrExecFailed}, nil
 	}
 
 	logrus.Debugf("Exiting Run() exitCode %d id=%s", exitCode, c.ID)

daemon/execdriver/windows/terminatekill.go

@@ -3,6 +3,9 @@
 package windows
 
 import (
+	"fmt"
+	"syscall"
+
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/daemon/execdriver"
 	"github.com/microsoft/hcsshim"
@@ -10,37 +13,36 @@ import (
 
 // Terminate implements the exec driver Driver interface.
 func (d *Driver) Terminate(p *execdriver.Command) error {
-	logrus.Debugf("WindowsExec: Terminate() id=%s", p.ID)
-	return kill(p.ID, p.ContainerPid)
+	return kill(p.ID, p.ContainerPid, syscall.SIGTERM)
 }
 
 // Kill implements the exec driver Driver interface.
 func (d *Driver) Kill(p *execdriver.Command, sig int) error {
-	logrus.Debugf("WindowsExec: Kill() id=%s sig=%d", p.ID, sig)
-	return kill(p.ID, p.ContainerPid)
+	return kill(p.ID, p.ContainerPid, syscall.Signal(sig))
 }
 
-func kill(id string, pid int) error {
-	logrus.Debugln("kill() ", id, pid)
+func kill(id string, pid int, sig syscall.Signal) error {
+	logrus.Debugf("WindowsExec: kill() id=%s pid=%d sig=%d", id, pid, sig)
 	var err error
+	context := fmt.Sprintf("kill: sig=%d pid=%d", sig, pid)
 
-	// Terminate Process
-	if err = hcsshim.TerminateProcessInComputeSystem(id, uint32(pid)); err != nil {
-		logrus.Warnf("Failed to terminate pid %d in %s: %q", pid, id, err)
-		// Ignore errors
-		err = nil
-	}
-
-	if terminateMode {
+	if sig == syscall.SIGKILL || forceKill {
 		// Terminate the compute system
-		if err = hcsshim.TerminateComputeSystem(id); err != nil {
-			logrus.Errorf("Failed to terminate %s - %q", id, err)
+		if errno, err := hcsshim.TerminateComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil {
+			logrus.Errorf("Failed to terminate %s - 0x%X %q", id, errno, err)
 		}
 
 	} else {
+		// Terminate Process
+		if err = hcsshim.TerminateProcessInComputeSystem(id, uint32(pid)); err != nil {
+			logrus.Warnf("Failed to terminate pid %d in %s: %q", pid, id, err)
+			// Ignore errors
+			err = nil
+		}
+
 		// Shutdown the compute system
-		if err = hcsshim.ShutdownComputeSystem(id); err != nil {
-			logrus.Errorf("Failed to shutdown %s - %q", id, err)
+		if errno, err := hcsshim.ShutdownComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil {
+			logrus.Errorf("Failed to shutdown %s - 0x%X %q", id, errno, err)
 		}
 	}
 	return err

daemon/execdriver/windows/windows.go

@@ -18,7 +18,8 @@ import (
 var dummyMode bool
 
 // This allows the daemon to terminate containers rather than shutdown
-var terminateMode bool
+// This allows the daemon to force kill (HCS terminate) rather than shutdown
+var forceKill bool
 
 // Define name and version for windows
 var (
@@ -62,11 +63,11 @@ func NewDriver(root, initPath string, options []string) (*Driver, error) {
 				logrus.Warn("Using dummy mode in Windows exec driver. This is for development use only!")
 			}
 
-		case "terminate":
+		case "forcekill":
 			switch val {
 			case "1":
-				terminateMode = true
-				logrus.Warn("Using terminate mode in Windows exec driver. This is for testing purposes only.")
+				forceKill = true
+				logrus.Warn("Using force kill mode in Windows exec driver. This is for testing purposes only.")
 			}
 
 		default: