X Tutup
Skip to content

Commit 61da698

Browse files
author
Akshat Kumar
committed
Cleanup open pipes if logging binary fails to start
Signed-off-by: Akshat Kumar <kshtku@amazon.com>
1 parent 4cc99e5 commit 61da698

File tree

4 files changed

+51
-3
lines changed

4 files changed

+51
-3
lines changed

‎runtime/io.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package runtime
1818

1919
import (
2020
"net/url"
21+
"os"
2122
"os/exec"
2223
)
2324

@@ -42,3 +43,11 @@ func NewBinaryCmd(binaryURI *url.URL, id, ns string) *exec.Cmd {
4243

4344
return cmd
4445
}
46+
47+
// CloseFiles closes any files passed in.
48+
// It it used for cleanup in the event of unexpected errors.
49+
func CloseFiles(files ...*os.File) {
50+
for _, file := range files {
51+
file.Close()
52+
}
53+
}

‎runtime/v1/shim/service_linux.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type linuxPlatform struct {
3535
epoller *console.Epoller
3636
}
3737

38-
func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (console.Console, error) {
38+
func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (cons console.Console, retErr error) {
3939
if p.epoller == nil {
4040
return nil, errors.New("uninitialized epoller")
4141
}
@@ -77,22 +77,34 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
7777

7878
cmd := runtime.NewBinaryCmd(uri, id, ns)
7979

80+
// In case of unexpected errors during logging binary start, close open pipes
81+
var filesToClose []*os.File
82+
83+
defer func() {
84+
if retErr != nil {
85+
runtime.CloseFiles(filesToClose...)
86+
}
87+
}()
88+
8089
// Create pipe to be used by logging binary for Stdout
8190
outR, outW, err := os.Pipe()
8291
if err != nil {
8392
return nil, errors.Wrap(err, "failed to create stdout pipes")
8493
}
94+
filesToClose = append(filesToClose, outR)
8595

8696
// Stderr is created for logging binary but unused when terminal is true
8797
serrR, _, err := os.Pipe()
8898
if err != nil {
8999
return nil, errors.Wrap(err, "failed to create stderr pipes")
90100
}
101+
filesToClose = append(filesToClose, serrR)
91102

92103
r, w, err := os.Pipe()
93104
if err != nil {
94105
return nil, err
95106
}
107+
filesToClose = append(filesToClose, r)
96108

97109
cmd.ExtraFiles = append(cmd.ExtraFiles, outR, serrR, w)
98110

@@ -119,6 +131,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
119131
if _, err := r.Read(b); err != nil && err != io.EOF {
120132
return nil, errors.Wrap(err, "failed to read from logging binary")
121133
}
134+
cwg.Wait()
122135

123136
default:
124137
outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0)

‎runtime/v1/shim/service_unix.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import (
3636
type unixPlatform struct {
3737
}
3838

39-
func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (console.Console, error) {
39+
func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (cons console.Console, retErr error) {
4040
var cwg sync.WaitGroup
4141
if stdin != "" {
4242
in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0)
@@ -66,22 +66,34 @@ func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console,
6666

6767
cmd := runtime.NewBinaryCmd(uri, id, ns)
6868

69+
// In case of unexpected errors during logging binary start, close open pipes
70+
var filesToClose []*os.File
71+
72+
defer func() {
73+
if retErr != nil {
74+
runtime.CloseFiles(filesToClose...)
75+
}
76+
}()
77+
6978
// Create pipe to be used by logging binary for Stdout
7079
outR, outW, err := os.Pipe()
7180
if err != nil {
7281
return nil, errors.Wrap(err, "failed to create stdout pipes")
7382
}
83+
filesToClose = append(filesToClose, outR)
7484

7585
// Stderr is created for logging binary but unused when terminal is true
7686
serrR, _, err := os.Pipe()
7787
if err != nil {
7888
return nil, errors.Wrap(err, "failed to create stderr pipes")
7989
}
90+
filesToClose = append(filesToClose, serrR)
8091

8192
r, w, err := os.Pipe()
8293
if err != nil {
8394
return nil, err
8495
}
96+
filesToClose = append(filesToClose, r)
8597

8698
cmd.ExtraFiles = append(cmd.ExtraFiles, outR, serrR, w)
8799

@@ -108,6 +120,7 @@ func (p *unixPlatform) CopyConsole(ctx context.Context, console console.Console,
108120
if _, err := r.Read(b); err != nil && err != io.EOF {
109121
return nil, errors.Wrap(err, "failed to read from logging binary")
110122
}
123+
cwg.Wait()
111124

112125
default:
113126
outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0)

‎runtime/v2/runc/platform.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type linuxPlatform struct {
5959
epoller *console.Epoller
6060
}
6161

62-
func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (console.Console, error) {
62+
func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console, id, stdin, stdout, stderr string, wg *sync.WaitGroup) (cons console.Console, retErr error) {
6363
if p.epoller == nil {
6464
return nil, errors.New("uninitialized epoller")
6565
}
@@ -101,22 +101,34 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
101101

102102
cmd := runtime.NewBinaryCmd(uri, id, ns)
103103

104+
// In case of unexpected errors during logging binary start, close open pipes
105+
var filesToClose []*os.File
106+
107+
defer func() {
108+
if retErr != nil {
109+
runtime.CloseFiles(filesToClose...)
110+
}
111+
}()
112+
104113
// Create pipe to be used by logging binary for Stdout
105114
outR, outW, err := os.Pipe()
106115
if err != nil {
107116
return nil, errors.Wrap(err, "failed to create stdout pipes")
108117
}
118+
filesToClose = append(filesToClose, outR)
109119

110120
// Stderr is created for logging binary but unused when terminal is true
111121
serrR, _, err := os.Pipe()
112122
if err != nil {
113123
return nil, errors.Wrap(err, "failed to create stderr pipes")
114124
}
125+
filesToClose = append(filesToClose, serrR)
115126

116127
r, w, err := os.Pipe()
117128
if err != nil {
118129
return nil, err
119130
}
131+
filesToClose = append(filesToClose, r)
120132

121133
cmd.ExtraFiles = append(cmd.ExtraFiles, outR, serrR, w)
122134

@@ -143,6 +155,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
143155
if _, err := r.Read(b); err != nil && err != io.EOF {
144156
return nil, errors.Wrap(err, "failed to read from logging binary")
145157
}
158+
cwg.Wait()
146159

147160
default:
148161
outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0)

0 commit comments

Comments
 (0)
X Tutup