From 9175401b283d6437db7ec6cd3541a018c51322b5 Mon Sep 17 00:00:00 2001
From: Maksym Pavlenko <makpav@amazon.com>
Date: Fri, 17 Apr 2020 15:56:21 -0700
Subject: [PATCH] Cleanup binary IO resources on error

Signed-off-by: Maksym Pavlenko <makpav@amazon.com>
---
 pkg/process/io.go | 46 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/pkg/process/io.go b/pkg/process/io.go
index 0603ffff0..914efd002 100644
--- a/pkg/process/io.go
+++ b/pkg/process/io.go
@@ -246,11 +246,12 @@ func (c *countingWriteCloser) Close() error {
 }
 
 // NewBinaryIO runs a custom binary process for pluggable shim logging
-func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (runc.IO, error) {
+func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (_ runc.IO, err error) {
 	ns, err := namespaces.NamespaceRequired(ctx)
 	if err != nil {
 		return nil, err
 	}
+
 	var args []string
 	for k, vs := range uri.Query() {
 		args = append(args, k)
@@ -259,20 +260,35 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (runc.IO, error)
 		}
 	}
 
+	var closers []func() error
+	defer func() {
+		if err == nil {
+			return
+		}
+		result := multierror.Append(err)
+		for _, fn := range closers {
+			result = multierror.Append(result, fn())
+		}
+		err = multierror.Flatten(result)
+	}()
+
 	out, err := newPipe()
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "failed to create stdout pipes")
 	}
+	closers = append(closers, out.Close)
 
 	serr, err := newPipe()
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "failed to create stderr pipes")
 	}
+	closers = append(closers, serr.Close)
 
 	r, w, err := os.Pipe()
 	if err != nil {
 		return nil, err
 	}
+	closers = append(closers, r.Close, w.Close)
 
 	cmd := exec.Command(uri.Path, args...)
 	cmd.Env = append(cmd.Env,
@@ -284,17 +300,21 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (runc.IO, error)
 	// don't need to register this with the reaper or wait when
 	// running inside a shim
 	if err := cmd.Start(); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "failed to start binary process")
 	}
+	closers = append(closers, func() error { return cmd.Process.Kill() })
+
 	// close our side of the pipe after start
 	if err := w.Close(); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "failed to close write pipe after start")
 	}
+
 	// wait for the logging binary to be ready
 	b := make([]byte, 1)
 	if _, err := r.Read(b); err != nil && err != io.EOF {
-		return nil, err
+		return nil, errors.Wrap(err, "failed to read from logging binary")
 	}
+
 	return &binaryIO{
 		cmd: cmd,
 		out: out,
@@ -423,9 +443,15 @@ type pipe struct {
 }
 
 func (p *pipe) Close() error {
-	err := p.w.Close()
-	if rerr := p.r.Close(); err == nil {
-		err = rerr
+	var result *multierror.Error
+
+	if err := p.w.Close(); err != nil {
+		result = multierror.Append(result, errors.Wrap(err, "failed to close write pipe"))
 	}
-	return err
+
+	if err := p.r.Close(); err != nil {
+		result = multierror.Append(result, errors.Wrap(err, "failed to close read pipe"))
+	}
+
+	return multierror.Prefix(result.ErrorOrNil(), "pipe:")
 }