From a342ac7e10862d4ea399218d534fcc72d4c069f0 Mon Sep 17 00:00:00 2001
From: Daniel Canter <dcanter@microsoft.com>
Date: Mon, 16 Aug 2021 05:11:45 -0700
Subject: [PATCH] Misc. job containers cleanup

This change does two things:

1. Checks if the stdio pipes are nil before closing them via the CloseStdout,
CloseStderr, CloseStdin methods. This just brings it inline with the other
`cow.Process` implementations that check for nilness.
2. Fix an oversight where windows.Close was being used instead of windows.FreeLibrary
after loading kernel32.dll

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
---
 internal/jobcontainers/process.go | 36 +++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/internal/jobcontainers/process.go b/internal/jobcontainers/process.go
index 2cccc8a1d3..45704adef3 100644
--- a/internal/jobcontainers/process.go
+++ b/internal/jobcontainers/process.go
@@ -92,21 +92,39 @@ func (p *JobProcess) Signal(ctx context.Context, options interface{}) (bool, err
 func (p *JobProcess) CloseStdin(ctx context.Context) error {
 	p.stdioLock.Lock()
 	defer p.stdioLock.Unlock()
-	return p.stdin.Close()
+	if p.stdin != nil {
+		if err := p.stdin.Close(); err != nil {
+			return errors.Wrap(err, "failed to close job container stdin")
+		}
+		p.stdin = nil
+	}
+	return nil
 }
 
 // CloseStdout closes the stdout pipe of the process.
 func (p *JobProcess) CloseStdout(ctx context.Context) error {
 	p.stdioLock.Lock()
 	defer p.stdioLock.Unlock()
-	return p.stdout.Close()
+	if p.stdout != nil {
+		if err := p.stdout.Close(); err != nil {
+			return errors.Wrap(err, "failed to close job container stdout")
+		}
+		p.stdout = nil
+	}
+	return nil
 }
 
 // CloseStderr closes the stderr pipe of the process.
 func (p *JobProcess) CloseStderr(ctx context.Context) error {
 	p.stdioLock.Lock()
 	defer p.stdioLock.Unlock()
-	return p.stderr.Close()
+	if p.stderr != nil {
+		if err := p.stderr.Close(); err != nil {
+			return errors.Wrap(err, "failed to close job container stderr")
+		}
+		p.stderr = nil
+	}
+	return nil
 }
 
 // Wait waits for the process to exit. If the process has already exited returns
@@ -217,7 +235,9 @@ func signalProcess(pid uint32, signal int) error {
 	if err != nil {
 		return errors.Wrap(err, "failed to open process")
 	}
-	defer windows.Close(hProc)
+	defer func() {
+		_ = windows.Close(hProc)
+	}()
 
 	// We can't use GenerateConsoleCtrlEvent since that only supports CTRL_C_EVENT and CTRL_BREAK_EVENT.
 	// Instead, to handle an arbitrary signal we open a CtrlRoutine thread inside the target process and
@@ -231,7 +251,9 @@ func signalProcess(pid uint32, signal int) error {
 	if err != nil {
 		return errors.Wrap(err, "failed to load kernel32 library")
 	}
-	defer windows.Close(k32)
+	defer func() {
+		_ = windows.FreeLibrary(k32)
+	}()
 
 	proc, err := windows.GetProcAddress(k32, "CtrlRoutine")
 	if err != nil {
@@ -242,6 +264,8 @@ func signalProcess(pid uint32, signal int) error {
 	if err != nil {
 		return errors.Wrapf(err, "failed to open remote thread in target process %d", pid)
 	}
-	defer windows.Close(threadHandle)
+	defer func() {
+		_ = windows.Close(threadHandle)
+	}()
 	return nil
 }