Cleanup open pipes if logging binary fails to start

Signed-off-by: Akshat Kumar <kshtku@amazon.com>
containerd · Sep 11, 2020 · 61da698 · 61da698
1 parent 4cc99e5
commit 61da698
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 3 deletions.
diff --git a/runtime/io.go b/runtime/io.go
@@ -18,6 +18,7 @@ package runtime
 
 import (
 	"net/url"
+	"os"
 	"os/exec"
 )
 
@@ -42,3 +43,11 @@ func NewBinaryCmd(binaryURI *url.URL, id, ns string) *exec.Cmd {
 
 	return cmd
 }
+
+// CloseFiles closes any files passed in.
+// It it used for cleanup in the event of unexpected errors.
+func CloseFiles(files ...*os.File) {
+	for _, file := range files {
+		file.Close()
+	}
+}
diff --git a/runtime/v1/shim/service_linux.go b/runtime/v1/shim/service_linux.go
@@ -35,7 +35,7 @@ type linuxPlatform struct {
 	epoller *console.Epoller
 }
 
-func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (console.Console, error) {
+func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (cons console.Console, retErr error) {
 	if p.epoller == nil {
 		return nil, errors.New("uninitialized epoller")
 	}
@@ -77,22 +77,34 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
 
 		cmd := runtime.NewBinaryCmd(uri, id, ns)
 
+		// In case of unexpected errors during logging binary start, close open pipes
+		var filesToClose []*os.File
+
+		defer func() {
+			if retErr != nil {
+				runtime.CloseFiles(filesToClose...)
+			}
+		}()
+
 		// Create pipe to be used by logging binary for Stdout
 		outR, outW, err := os.Pipe()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create stdout pipes")
 		}
+		filesToClose = append(filesToClose, outR)
 
 		// Stderr is created for logging binary but unused when terminal is true
 		serrR, _, err := os.Pipe()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create stderr pipes")
 		}
+		filesToClose = append(filesToClose, serrR)
 
 		r, w, err := os.Pipe()
 		if err != nil {
 			return nil, err
 		}
+		filesToClose = append(filesToClose, r)
 
 		cmd.ExtraFiles = append(cmd.ExtraFiles, outR, serrR, w)
 
@@ -119,6 +131,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
 		if _, err := r.Read(b); err != nil && err != io.EOF {
 			return nil, errors.Wrap(err, "failed to read from logging binary")
 		}
+		cwg.Wait()
 
 	default:
 		outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0)

diff --git a/runtime/v1/shim/service_unix.go b/runtime/v1/shim/service_unix.go
@@ -36,7 +36,7 @@ import (
 type unixPlatform struct {
 }
 
-func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (console.Console, error) {
+func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (cons console.Console, retErr error) {
 	var cwg sync.WaitGroup
 	if stdin != "" {
 		in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0)
@@ -66,22 +66,34 @@ func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console,
 
 		cmd := runtime.NewBinaryCmd(uri, id, ns)
 
+		// In case of unexpected errors during logging binary start, close open pipes
+		var filesToClose []*os.File
+
+		defer func() {
+			if retErr != nil {
+				runtime.CloseFiles(filesToClose...)
+			}
+		}()
+
 		// Create pipe to be used by logging binary for Stdout
 		outR, outW, err := os.Pipe()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create stdout pipes")
 		}
+		filesToClose = append(filesToClose, outR)
 
 		// Stderr is created for logging binary but unused when terminal is true
 		serrR, _, err := os.Pipe()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create stderr pipes")
 		}
+		filesToClose = append(filesToClose, serrR)
 
 		r, w, err := os.Pipe()
 		if err != nil {
 			return nil, err
 		}
+		filesToClose = append(filesToClose, r)
 
 		cmd.ExtraFiles = append(cmd.ExtraFiles, outR, serrR, w)
 
@@ -108,6 +120,7 @@ func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console,
 		if _, err := r.Read(b); err != nil && err != io.EOF {
 			return nil, errors.Wrap(err, "failed to read from logging binary")
 		}
+		cwg.Wait()
 
 	default:
 		outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0)

diff --git a/runtime/v2/runc/platform.go b/runtime/v2/runc/platform.go
@@ -59,7 +59,7 @@ type linuxPlatform struct {
 	epoller *console.Epoller
 }
 
-func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (console.Console, error) {
+func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (cons console.Console, retErr error) {
 	if p.epoller == nil {
 		return nil, errors.New("uninitialized epoller")
 	}
@@ -101,22 +101,34 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
 
 		cmd := runtime.NewBinaryCmd(uri, id, ns)
 
+		// In case of unexpected errors during logging binary start, close open pipes
+		var filesToClose []*os.File
+
+		defer func() {
+			if retErr != nil {
+				runtime.CloseFiles(filesToClose...)
+			}
+		}()
+
 		// Create pipe to be used by logging binary for Stdout
 		outR, outW, err := os.Pipe()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create stdout pipes")
 		}
+		filesToClose = append(filesToClose, outR)
 
 		// Stderr is created for logging binary but unused when terminal is true
 		serrR, _, err := os.Pipe()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create stderr pipes")
 		}
+		filesToClose = append(filesToClose, serrR)
 
 		r, w, err := os.Pipe()
 		if err != nil {
 			return nil, err
 		}
+		filesToClose = append(filesToClose, r)
 
 		cmd.ExtraFiles = append(cmd.ExtraFiles, outR, serrR, w)
 
@@ -143,6 +155,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
 		if _, err := r.Read(b); err != nil && err != io.EOF {
 			return nil, errors.Wrap(err, "failed to read from logging binary")
 		}
+		cwg.Wait()
 
 	default:
 		outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0)