Merge pull request #2971 from jterry75/runhcs_shim_exec_deadlock

crosbymichael · web-flow · commit 18a8a061cc1d · 2019-02-01T14:34:57.000-05:00
Fix deadlock in Windows runhcs shim exec
diff --git a/runtime/v2/runhcs/process.go b/runtime/v2/runhcs/process.go
@@ -53,13 +53,17 @@ func newProcess(ctx context.Context, s *service, id string, pid uint32, pr *pipe
 		relay:     pr,
 		waitBlock: make(chan struct{}),
 	}
+	process.startedWg.Add(1)
 	go waitForProcess(ctx, process, p, s)
 	return process, nil
 }
 
+// waitForProcess waits for `p` to exit.
+//
+// The caller of `waitForProcess` MUST have incremented `process.startedWg` to
+// synchronize event start/exit publishing.
 func waitForProcess(ctx context.Context, process *process, p *os.Process, s *service) {
 	pid := uint32(p.Pid)
-	process.startedWg.Add(1)
 
 	// Store the default non-exited value for calls to stat
 	process.exit.Store(&processExit{
@@ -121,7 +125,6 @@ func newExecProcess(ctx context.Context, s *service, cid, id string, pr *pipeRel
 		relay:     pr,
 		waitBlock: make(chan struct{}),
 	}
-	process.startedWg.Add(1)
 
 	// Store the default non-exited value for calls to stat
 	process.exit.Store(&processExit{
diff --git a/runtime/v2/runhcs/service.go b/runtime/v2/runhcs/service.go
@@ -682,6 +682,17 @@ func (s *service) Start(ctx context.Context, r *taskAPI.StartRequest) (*taskAPI.
 			}
 		}
 
+		// For the exec case increment the startedWg to make sure we publish the
+		// started event previous to the exit published in `waitForProcess`.
+		p.startedWg.Add(1)
+		defer func() {
+			if err != nil {
+				// If an exec failure takes place decrement the startedWg so we
+				// dont get off on counts.
+				p.startedWg.Done()
+			}
+		}()
+
 		// ID here is the containerID to exec the process in.
 		err = rhcs.Exec(ctx, r.ID, procConfig, eopts)
 		if err != nil {
