Merge pull request #37583 from Microsoft/jjh.37562

Don't invoke HCS shutdown if terminate called; removes unused vars and no-op calls

libcontainerd/client_local_windows.go

@@ -42,18 +42,17 @@ type container struct {
 	// have access to the Spec
 	ociSpec *specs.Spec
 
-	isWindows           bool
+	isWindows    bool
-	manualStopRequested bool
+	hcsContainer hcsshim.Container
-	hcsContainer        hcsshim.Container
 
-	id            string
+	id               string
-	status        Status
+	status           Status
-	exitedAt      time.Time
+	exitedAt         time.Time
-	exitCode      uint32
+	exitCode         uint32
-	waitCh        chan struct{}
+	waitCh           chan struct{}
-	init          *process
+	init             *process
-	execs         map[string]*process
+	execs            map[string]*process
-	updatePending bool
+	terminateInvoked bool
 }
 
 // Win32 error codes that are used for various workarounds
@@ -324,15 +323,15 @@ func (c *client) createWindows(id string, spec *specs.Spec, runtimeOptions inter
 	logger.Debug("starting container")
 	if err = hcsContainer.Start(); err != nil {
 		c.logger.WithError(err).Error("failed to start container")
-		ctr.debugGCS()
+		ctr.Lock()
 		if err := c.terminateContainer(ctr); err != nil {
 			c.logger.WithError(err).Error("failed to cleanup after a failed Start")
 		} else {
 			c.logger.Debug("cleaned up after failed Start by calling Terminate")
 		}
+		ctr.Unlock()
 		return err
 	}
-	ctr.debugGCS()
 
 	c.Lock()
 	c.containers[id] = ctr
@@ -528,11 +527,13 @@ func (c *client) createLinux(id string, spec *specs.Spec, runtimeOptions interfa
 	if err = hcsContainer.Start(); err != nil {
 		c.logger.WithError(err).Error("failed to start container")
 		ctr.debugGCS()
+		ctr.Lock()
 		if err := c.terminateContainer(ctr); err != nil {
 			c.logger.WithError(err).Error("failed to cleanup after a failed Start")
 		} else {
 			c.logger.Debug("cleaned up after failed Start by calling Terminate")
 		}
+		ctr.Unlock()
 		return err
 	}
 	ctr.debugGCS()
@@ -852,8 +853,6 @@ func (c *client) SignalProcess(_ context.Context, containerID, processID string,
 		return err
 	}
 
-	ctr.manualStopRequested = true
-
 	logger := c.logger.WithFields(logrus.Fields{
 		"container": containerID,
 		"process":   processID,
@@ -865,11 +864,14 @@ func (c *client) SignalProcess(_ context.Context, containerID, processID string,
 	if processID == InitProcessName {
 		if syscall.Signal(signal) == syscall.SIGKILL {
 			// Terminate the compute system
+			ctr.Lock()
+			ctr.terminateInvoked = true
 			if err := ctr.hcsContainer.Terminate(); err != nil {
 				if !hcsshim.IsPending(err) {
 					logger.WithError(err).Error("failed to terminate hccshim container")
 				}
 			}
+			ctr.Unlock()
 		} else {
 			// Shut down the container
 			if err := ctr.hcsContainer.Shutdown(); err != nil {
@@ -1171,12 +1173,17 @@ func (c *client) getProcess(containerID, processID string) (*container, *process
 	return ctr, p, nil
 }
 
+// ctr mutex must be held when calling this function.
 func (c *client) shutdownContainer(ctr *container) error {
-	const shutdownTimeout = time.Minute * 5
+	var err error
-	err := ctr.hcsContainer.Shutdown()
+	const waitTimeout = time.Minute * 5
 
-	if hcsshim.IsPending(err) {
+	if !ctr.terminateInvoked {
-		err = ctr.hcsContainer.WaitTimeout(shutdownTimeout)
+		err = ctr.hcsContainer.Shutdown()
+	}
+
+	if hcsshim.IsPending(err) || ctr.terminateInvoked {
+		err = ctr.hcsContainer.WaitTimeout(waitTimeout)
 	} else if hcsshim.IsAlreadyStopped(err) {
 		err = nil
 	}
@@ -1196,8 +1203,10 @@ func (c *client) shutdownContainer(ctr *container) error {
 	return nil
 }
 
+// ctr mutex must be held when calling this function.
 func (c *client) terminateContainer(ctr *container) error {
 	const terminateTimeout = time.Minute * 5
+	ctr.terminateInvoked = true
 	err := ctr.hcsContainer.Terminate()
 
 	if hcsshim.IsPending(err) {
@@ -1263,7 +1272,6 @@ func (c *client) reapProcess(ctr *container, p *process) int {
 		ctr.exitedAt = exitedAt
 		ctr.exitCode = uint32(exitCode)
 		close(ctr.waitCh)
-		ctr.Unlock()
 
 		if err := c.shutdownContainer(ctr); err != nil {
 			exitCode = -1
@@ -1277,6 +1285,7 @@ func (c *client) reapProcess(ctr *container, p *process) int {
 		} else {
 			logger.Debug("completed container shutdown")
 		}
+		ctr.Unlock()
 
 		if err := ctr.hcsContainer.Close(); err != nil {
 			exitCode = -1