Skip to content

Commit 0eaa502

Browse files
ethan-lowman-ddethan-lowman-dd
committed
try to avoid orphans on windows
Signed-off-by: Ethan Lowman <[email protected]>
1 parent 4a4b4e2 commit 0eaa502

4 files changed

Lines changed: 104 additions & 16 deletions

File tree

‎pkg/imageverifier/bindir/bindir.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,9 @@ func (v *ImageVerifier) runVerifier(ctx context.Context, bin string, imageName s
122122
}
123123

124124
cmd := exec.CommandContext(ctx, binPath, args...)
125-
configureVerifierCommand(cmd)
126125

127-
// We construct our own pipes instead of using cmd.StdinPipe, cmd.StoutPipe,
128-
// and cmd.StderrPipe in order to set timeouts on reads and writes.
126+
// We construct our own pipes instead of using the default StdinPipe,
127+
// StoutPipe, and StderrPipe in order to set timeouts on reads and writes.
129128
stdinRead, stdinWrite, err := os.Pipe()
130129
if err != nil {
131130
return -1, "", err
@@ -136,15 +135,15 @@ func (v *ImageVerifier) runVerifier(ctx context.Context, bin string, imageName s
136135

137136
stdoutRead, stdoutWrite, err := os.Pipe()
138137
if err != nil {
139-
return -1, "", fmt.Errorf("creating stdout pipe: %w", err)
138+
return -1, "", err
140139
}
141140
cmd.Stdout = stdoutWrite
142141
defer stdoutRead.Close()
143142
defer stdoutWrite.Close()
144143

145144
stderrRead, stderrWrite, err := os.Pipe()
146145
if err != nil {
147-
return -1, "", fmt.Errorf("creating stderr pipe: %w", err)
146+
return -1, "", err
148147
}
149148
cmd.Stderr = stderrWrite
150149
defer stderrRead.Close()
@@ -158,10 +157,12 @@ func (v *ImageVerifier) runVerifier(ctx context.Context, bin string, imageName s
158157
stderrRead.SetDeadline(d)
159158
}
160159

161-
// Fork & exec the child process.
162-
if err := cmd.Start(); err != nil {
163-
return -1, "", fmt.Errorf("starting process: %w", err)
160+
// Finish configuring, and then fork & exec the child process.
161+
p, err := startProcess(ctx, cmd)
162+
if err != nil {
163+
return -1, "", err
164164
}
165+
defer p.cleanup(ctx)
165166

166167
// Close the child ends of the pipes in the parent process.
167168
stdinRead.Close()

‎pkg/imageverifier/bindir/bindir_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ func TestBinDirVerifyImage(t *testing.T) {
369369
t.Fatal(err)
370370
}
371371

372-
if strings.Contains(string(b), "-sleep-forever") {
372+
if strings.Contains(string(b), "verifier-") {
373373
t.Fatalf("killing the verifier binary didn't kill all its children:\n%v", string(b))
374374
}
375375
})

‎pkg/imageverifier/bindir/processes_unix.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,20 @@
1919
package bindir
2020

2121
import (
22+
"context"
23+
"fmt"
2224
"os/exec"
2325

2426
"golang.org/x/sys/unix"
2527
)
2628

27-
func configureVerifierCommand(cmd *exec.Cmd) {
28-
// Configure the verifier command so that killing it kills all child
29-
// processes of the verifier process.
29+
type process struct {
30+
cmd *exec.Cmd
31+
}
3032

33+
// Configure the verifier command so that killing it kills all child
34+
// processes of the verifier process.
35+
func startProcess(ctx context.Context, cmd *exec.Cmd) (*process, error) {
3136
// Assign the verifier a new process group so that killing its process group
3237
// in Cancel() doesn't kill the parent process (containerd).
3338
cmd.SysProcAttr = &unix.SysProcAttr{Setpgid: true}
@@ -37,4 +42,14 @@ func configureVerifierCommand(cmd *exec.Cmd) {
3742
// process group whose ID is cmd.Process.Pid.
3843
return unix.Kill(-cmd.Process.Pid, unix.SIGKILL)
3944
}
45+
46+
if err := cmd.Start(); err != nil {
47+
return nil, fmt.Errorf("starting process: %w", err)
48+
}
49+
50+
return &process{
51+
cmd: cmd,
52+
}, nil
4053
}
54+
55+
func (p *process) cleanup(ctx context.Context) {}

‎pkg/imageverifier/bindir/processes_windows.go‎

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,83 @@
1919
package bindir
2020

2121
import (
22+
"context"
23+
"fmt"
2224
"os/exec"
25+
"unsafe"
26+
27+
"github.com/containerd/containerd/log"
28+
"golang.org/x/sys/windows"
2329
)
2430

25-
func configureVerifierCommand(cmd *exec.Cmd) {
26-
// No customizations needed for Windows since the orphaned process leak
27-
// mitigated in the Unix implementation of configureVerifierCommand is not
28-
// applicable on Windows.
31+
type process struct {
32+
cmd *exec.Cmd
33+
34+
jobHandle *windows.Handle
35+
processHandle *windows.Handle
36+
}
37+
38+
// Configure the verifier command so that killing it kills all child
39+
// processes of the verifier process.
40+
//
41+
// Job/process management based on:
42+
// https://devblogs.microsoft.com/oldnewthing/20131209-00/?p=2433
43+
func startProcess(ctx context.Context, cmd *exec.Cmd) (*process, error) {
44+
p := &process{
45+
cmd: cmd,
46+
}
47+
48+
jobHandle, err := windows.CreateJobObject(nil, nil)
49+
if err != nil {
50+
return nil, fmt.Errorf("creating job object: %w", err)
51+
}
52+
p.jobHandle = &jobHandle
53+
54+
info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{
55+
BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{
56+
LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE,
57+
},
58+
}
59+
_, err = windows.SetInformationJobObject(
60+
jobHandle,
61+
windows.JobObjectExtendedLimitInformation,
62+
uintptr(unsafe.Pointer(&info)),
63+
uint32(unsafe.Sizeof(info)),
64+
)
65+
if err != nil {
66+
p.cleanup(ctx)
67+
return nil, fmt.Errorf("setting limits for job object: %w", err)
68+
}
69+
70+
if err := cmd.Start(); err != nil {
71+
p.cleanup(ctx)
72+
return nil, fmt.Errorf("starting process: %w", err)
73+
}
74+
75+
processHandle, err := windows.OpenProcess(windows.PROCESS_QUERY_INFORMATION, false, uint32(cmd.Process.Pid))
76+
if err != nil {
77+
return nil, fmt.Errorf("getting handle for verifier process: %w", err)
78+
}
79+
p.processHandle = &processHandle
80+
81+
err = windows.AssignProcessToJobObject(jobHandle, processHandle)
82+
if err != nil {
83+
p.cleanup(ctx)
84+
return nil, fmt.Errorf("associating new process to job object: %w", err)
85+
}
86+
87+
return p, nil
88+
}
89+
90+
func (p *process) cleanup(ctx context.Context) {
91+
if p.jobHandle != nil {
92+
if err := windows.CloseHandle(*p.jobHandle); err != nil {
93+
log.G(ctx).WithError(err).Error("failed to close job handle")
94+
}
95+
}
96+
if p.processHandle != nil {
97+
if err := windows.CloseHandle(*p.processHandle); err != nil {
98+
log.G(ctx).WithError(err).Error("failed to close process handle")
99+
}
100+
}
29101
}

0 commit comments

Comments
 (0)