cio.copyIO: fix pipes potentially not being closed (Windows)

The defer functions were checking the local variable, and would therefore
not be executed, as the function returned if an error occurred.

Perhaps best illustrated when renaming the local variables;

    if fifos.Stdin != "" {
        l, err1 := winio.ListenPipe(fifos.Stdin, nil)
        if err1 != nil {
            return nil, errors.Wrapf(err1, "failed to create stdin pipe %s", fifos.Stdin)
        }
        defer func(l net.Listener) {
            if err1 != nil {
                l.Close()
            }
        }(l)
        // ...
    }

    if fifos.Stdout != "" {
        l, err2 := winio.ListenPipe(fifos.Stdout, nil)
        if err2 != nil {
            return nil, errors.Wrapf(err2, "failed to create stdout pipe %s", fifos.Stdout)
        }
        defer func(l net.Listener) {
            if err2 != nil {
                l.Close()
            }
        }(l)
        // ....
    }

This patch changes the function to use a named return variable, and to use
a single `defer()` that closes all pipes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2021-01-29 12:31:46 +01:00
parent baf6c1d5e2
commit 219fa3d0a5
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C

View File

@ -20,7 +20,6 @@ import (
"context" "context"
"fmt" "fmt"
"io" "io"
"net"
winio "github.com/Microsoft/go-winio" winio "github.com/Microsoft/go-winio"
"github.com/containerd/containerd/log" "github.com/containerd/containerd/log"
@ -43,21 +42,28 @@ func NewFIFOSetInDir(_, id string, terminal bool) (*FIFOSet, error) {
}, nil), nil }, nil), nil
} }
func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) { func copyIO(fifos *FIFOSet, ioset *Streams) (_ *cio, retErr error) {
var ( var (
set []io.Closer set []io.Closer
) )
defer func() {
if retErr == nil {
return
}
for _, closer := range set {
if closer == nil {
continue
}
_ = closer.Close()
}
}()
if fifos.Stdin != "" { if fifos.Stdin != "" {
l, err := winio.ListenPipe(fifos.Stdin, nil) l, err := winio.ListenPipe(fifos.Stdin, nil)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to create stdin pipe %s", fifos.Stdin) return nil, errors.Wrapf(err, "failed to create stdin pipe %s", fifos.Stdin)
} }
defer func(l net.Listener) {
if err != nil {
l.Close()
}
}(l)
set = append(set, l) set = append(set, l)
go func() { go func() {
@ -81,11 +87,6 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to create stdout pipe %s", fifos.Stdout) return nil, errors.Wrapf(err, "failed to create stdout pipe %s", fifos.Stdout)
} }
defer func(l net.Listener) {
if err != nil {
l.Close()
}
}(l)
set = append(set, l) set = append(set, l)
go func() { go func() {
@ -109,11 +110,6 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to create stderr pipe %s", fifos.Stderr) return nil, errors.Wrapf(err, "failed to create stderr pipe %s", fifos.Stderr)
} }
defer func(l net.Listener) {
if err != nil {
l.Close()
}
}(l)
set = append(set, l) set = append(set, l)
go func() { go func() {