Fix race condition in legacy process stdio code

container.go

@@ -196,7 +196,7 @@ func (container *container) MappedVirtualDisks() (map[int]MappedVirtualDiskContr
 
 // CreateProcess launches a new process within the container.
 func (container *container) CreateProcess(c *ProcessConfig) (Process, error) {
-	p, err := container.system.CreateProcessNoStdio(c)
+	p, err := container.system.CreateProcess(context.Background(), c)
 	if err != nil {
 		return nil, convertSystemError(err, container)
 	}

internal/hcs/process.go

@@ -20,6 +20,8 @@ type Process struct {
 	handle         vmcompute.HcsProcess
 	processID      int
 	system         *System
+	hasCachedStdio bool
+	stdioLock      sync.Mutex
 	stdin          io.WriteCloser
 	stdout         io.ReadCloser
 	stderr         io.ReadCloser
@@ -272,8 +274,8 @@ func (process *Process) ExitCode() (int, error) {
 }
 
 // StdioLegacy returns the stdin, stdout, and stderr pipes, respectively. Closing
-// these pipes does not close the underlying pipes; but this function can only
-// be called once on each Process.
+// these pipes does not close the underlying pipes. Once returned, these pipes
+// are the responsibility of the caller to close.
 func (process *Process) StdioLegacy() (_ io.WriteCloser, _ io.ReadCloser, _ io.ReadCloser, err error) {
 	operation := "hcsshim::Process::StdioLegacy"
 	ctx, span := trace.StartSpan(context.Background(), operation)
@@ -290,6 +292,15 @@ func (process *Process) StdioLegacy() (_ io.WriteCloser, _ io.ReadCloser, _ io.R
 		return nil, nil, nil, makeProcessError(process, operation, ErrAlreadyClosed, nil)
 	}
 
+	process.stdioLock.Lock()
+	defer process.stdioLock.Unlock()
+	if process.hasCachedStdio {
+		stdin, stdout, stderr := process.stdin, process.stdout, process.stderr
+		process.stdin, process.stdout, process.stderr = nil, nil, nil
+		process.hasCachedStdio = false
+		return stdin, stdout, stderr, nil
+	}
+
 	processInfo, resultJSON, err := vmcompute.HcsGetProcessInfo(ctx, process.handle)
 	events := processHcsResult(ctx, resultJSON)
 	if err != nil {
@@ -307,6 +318,8 @@ func (process *Process) StdioLegacy() (_ io.WriteCloser, _ io.ReadCloser, _ io.R
 // Stdio returns the stdin, stdout, and stderr pipes, respectively.
 // To close them, close the process handle.
 func (process *Process) Stdio() (stdin io.Writer, stdout, stderr io.Reader) {
+	process.stdioLock.Lock()
+	defer process.stdioLock.Unlock()
 	return process.stdin, process.stdout, process.stderr
 }
 
@@ -340,9 +353,13 @@ func (process *Process) CloseStdin(ctx context.Context) error {
 		return makeProcessError(process, operation, err, events)
 	}
 
+	process.stdioLock.Lock()
 	if process.stdin != nil {
 		process.stdin.Close()
+		process.stdin = nil
 	}
+	process.stdioLock.Unlock()
+
 	return nil
 }
 
@@ -365,15 +382,20 @@ func (process *Process) Close() (err error) {
 		return nil
 	}
 
+	process.stdioLock.Lock()
 	if process.stdin != nil {
 		process.stdin.Close()
+		process.stdin = nil
 	}
 	if process.stdout != nil {
 		process.stdout.Close()
+		process.stdout = nil
 	}
 	if process.stderr != nil {
 		process.stderr.Close()
+		process.stderr = nil
 	}
+	process.stdioLock.Unlock()
 
 	if err = process.unregisterCallback(ctx); err != nil {
 		return makeProcessError(process, operation, err, nil)

internal/hcs/system.go

@@ -482,38 +482,6 @@ func (computeSystem *System) createProcess(ctx context.Context, operation string
 	return newProcess(processHandle, int(processInfo.ProcessId), computeSystem), &processInfo, nil
 }
 
-// CreateProcessNoStdio launches a new process within the computeSystem. The
-// Stdio handles are not cached on the process struct.
-func (computeSystem *System) CreateProcessNoStdio(c interface{}) (_ cow.Process, err error) {
-	operation := "hcsshim::System::CreateProcessNoStdio"
-	ctx, span := trace.StartSpan(context.Background(), operation)
-	defer span.End()
-	defer func() { oc.SetSpanStatus(span, err) }()
-	span.AddAttributes(trace.StringAttribute("cid", computeSystem.id))
-
-	process, processInfo, err := computeSystem.createProcess(ctx, operation, c)
-	if err != nil {
-		return nil, err
-	}
-	defer func() {
-		if err != nil {
-			process.Close()
-		}
-	}()
-
-	// We don't do anything with these handles. Close them so they don't leak.
-	syscall.Close(processInfo.StdInput)
-	syscall.Close(processInfo.StdOutput)
-	syscall.Close(processInfo.StdError)
-
-	if err = process.registerCallback(ctx); err != nil {
-		return nil, makeSystemError(computeSystem, operation, "", err, nil)
-	}
-	go process.waitBackground()
-
-	return process, nil
-}
-
 // CreateProcess launches a new process within the computeSystem.
 func (computeSystem *System) CreateProcess(ctx context.Context, c interface{}) (cow.Process, error) {
 	operation := "hcsshim::System::CreateProcess"
@@ -534,6 +502,7 @@ func (computeSystem *System) CreateProcess(ctx context.Context, c interface{}) (
 	process.stdin = pipes[0]
 	process.stdout = pipes[1]
 	process.stderr = pipes[2]
+	process.hasCachedStdio = true
 
 	if err = process.registerCallback(ctx); err != nil {
 		return nil, makeSystemError(computeSystem, operation, "", err, nil)