Skip to content

Commit 719a2c5

Browse files
crosbymichaelcrosbymichael
committed
Robust pid locking for shim processes
Closes containerd#2832 Signed-off-by: Michael Crosby <[email protected]>
1 parent bb9616b commit 719a2c5

5 files changed

Lines changed: 35 additions & 9 deletions

File tree

‎runtime/v1/linux/proc/exec.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"golang.org/x/sys/unix"
3232

3333
"github.com/containerd/console"
34+
"github.com/containerd/containerd/errdefs"
3435
"github.com/containerd/containerd/runtime/proc"
3536
"github.com/containerd/fifo"
3637
runc "github.com/containerd/go-runc"
@@ -49,7 +50,7 @@ type execProcess struct {
4950
io *processIO
5051
status int
5152
exited time.Time
52-
pid *safePid
53+
pid safePid
5354
closers []io.Closer
5455
stdin io.Closer
5556
stdio proc.Stdio
@@ -95,6 +96,7 @@ func (e *execProcess) setExited(status int) {
9596
e.status = status
9697
e.exited = time.Now()
9798
e.parent.Platform.ShutdownConsole(context.Background(), e.console)
99+
e.pid.set(StoppedPID)
98100
close(e.waitBlock)
99101
}
100102

@@ -142,7 +144,12 @@ func (e *execProcess) Kill(ctx context.Context, sig uint32, _ bool) error {
142144

143145
func (e *execProcess) kill(ctx context.Context, sig uint32, _ bool) error {
144146
pid := e.pid.get()
145-
if pid != 0 {
147+
switch {
148+
case pid == 0:
149+
return errors.Wrap(errdefs.ErrFailedPrecondition, "process not created")
150+
case pid < 0:
151+
return errors.Wrapf(errdefs.ErrNotFound, "process already finished")
152+
default:
146153
if err := unix.Kill(pid, syscall.Signal(sig)); err != nil {
147154
return errors.Wrapf(checkKillError(err), "exec kill error")
148155
}
@@ -254,10 +261,13 @@ func (e *execProcess) Status(ctx context.Context) (string, error) {
254261
}
255262
e.mu.Lock()
256263
defer e.mu.Unlock()
257-
// if we don't have a pid then the exec process has just been created
264+
// if we don't have a pid(pid=0) then the exec process has just been created
258265
if e.pid.get() == 0 {
259266
return "created", nil
260267
}
268+
if e.pid.get() == StoppedPID {
269+
return "stopped", nil
270+
}
261271
// if we have a pid and it can be signaled, the process is running
262272
if err := unix.Kill(e.pid.get(), 0); err == nil {
263273
return "running", nil

‎runtime/v1/linux/proc/init.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ type Init struct {
6464
runtime *runc.Runc
6565
status int
6666
exited time.Time
67-
pid int
67+
pid safePid
6868
closers []io.Closer
6969
stdin io.Closer
7070
stdio proc.Stdio
@@ -113,6 +113,9 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
113113
pio *processIO
114114
pidFile = newPidFile(p.Bundle)
115115
)
116+
p.pid.Lock()
117+
defer p.pid.Unlock()
118+
116119
if r.Terminal {
117120
if socket, err = runc.NewTempConsoleSocket(); err != nil {
118121
return errors.Wrap(err, "failed to create OCI runtime console socket")
@@ -167,7 +170,7 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
167170
if err != nil {
168171
return errors.Wrap(err, "failed to retrieve OCI runtime container pid")
169172
}
170-
p.pid = pid
173+
p.pid.pid = pid
171174
return nil
172175
}
173176

@@ -213,7 +216,7 @@ func (p *Init) ID() string {
213216

214217
// Pid of the process
215218
func (p *Init) Pid() int {
216-
return p.pid
219+
return p.pid.get()
217220
}
218221

219222
// ExitStatus of the process
@@ -272,6 +275,7 @@ func (p *Init) setExited(status int) {
272275
p.exited = time.Now()
273276
p.status = status
274277
p.Platform.ShutdownConsole(context.Background(), p.console)
278+
p.pid.set(StoppedPID)
275279
close(p.waitBlock)
276280
}
277281

@@ -405,7 +409,6 @@ func (p *Init) exec(ctx context.Context, path string, r *ExecConfig) (proc.Proce
405409
Terminal: r.Terminal,
406410
},
407411
waitBlock: make(chan struct{}),
408-
pid: &safePid{},
409412
}
410413
e.execState = &execCreatedState{p: e}
411414
return e, nil

‎runtime/v1/linux/proc/init_state.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ func (s *createdCheckpointState) Start(ctx context.Context) error {
143143
p := s.p
144144
sio := p.stdio
145145

146+
p.pid.Lock()
147+
defer p.pid.Unlock()
148+
146149
var (
147150
err error
148151
socket *runc.Socket
@@ -182,7 +185,7 @@ func (s *createdCheckpointState) Start(ctx context.Context) error {
182185
if err != nil {
183186
return errors.Wrap(err, "failed to retrieve OCI runtime container pid")
184187
}
185-
p.pid = pid
188+
p.pid.pid = pid
186189
return s.transition("running")
187190
}
188191

‎runtime/v1/linux/proc/process.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ import (
2323
)
2424

2525
// RuncRoot is the path to the root runc state directory
26-
const RuncRoot = "/run/containerd/runc"
26+
const (
27+
RuncRoot = "/run/containerd/runc"
28+
// StoppedPID is the pid assigned after a container has run and stopped
29+
StoppedPID = -1
30+
)
2731

2832
func stateName(v interface{}) string {
2933
switch v.(type) {

‎runtime/v1/linux/proc/utils.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ func (s *safePid) get() int {
4747
return s.pid
4848
}
4949

50+
func (s *safePid) set(pid int) {
51+
s.Lock()
52+
s.pid = pid
53+
s.Unlock()
54+
}
55+
5056
// TODO(mlaventure): move to runc package?
5157
func getLastRuntimeError(r *runc.Runc) (string, error) {
5258
if r.Log == "" {

0 commit comments

Comments
 (0)