Skip to content

Commit f71f6d3

Browse files
crosbymichaelthaJeztah
crosbymichael
authored and
thaJeztah
committed
Robust pid locking for shim processes
Closes #2832 Signed-off-by: Michael Crosby <[email protected]> (cherry picked from commit 719a2c5) Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 42aba6e commit f71f6d3

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 runc.IO
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
}
@@ -248,10 +255,13 @@ func (e *execProcess) Status(ctx context.Context) (string, error) {
248255
}
249256
e.mu.Lock()
250257
defer e.mu.Unlock()
251-
// if we don't have a pid then the exec process has just been created
258+
// if we don't have a pid(pid=0) then the exec process has just been created
252259
if e.pid.get() == 0 {
253260
return "created", nil
254261
}
262+
if e.pid.get() == StoppedPID {
263+
return "stopped", nil
264+
}
255265
// if we have a pid and it can be signaled, the process is running
256266
if err := unix.Kill(e.pid.get(), 0); err == nil {
257267
return "running", nil

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ type Init struct {
6767
runtime *runc.Runc
6868
status int
6969
exited time.Time
70-
pid int
70+
pid safePid
7171
closers []io.Closer
7272
stdin io.Closer
7373
stdio proc.Stdio
@@ -113,6 +113,9 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
113113
err error
114114
socket *runc.Socket
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")
@@ -191,7 +194,7 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
191194
if err != nil {
192195
return errors.Wrap(err, "failed to retrieve OCI runtime container pid")
193196
}
194-
p.pid = pid
197+
p.pid.pid = pid
195198
return nil
196199
}
197200

@@ -207,7 +210,7 @@ func (p *Init) ID() string {
207210

208211
// Pid of the process
209212
func (p *Init) Pid() int {
210-
return p.pid
213+
return p.pid.get()
211214
}
212215

213216
// ExitStatus of the process
@@ -266,6 +269,7 @@ func (p *Init) setExited(status int) {
266269
p.exited = time.Now()
267270
p.status = status
268271
p.Platform.ShutdownConsole(context.Background(), p.console)
272+
p.pid.set(StoppedPID)
269273
close(p.waitBlock)
270274
}
271275

@@ -408,7 +412,6 @@ func (p *Init) exec(ctx context.Context, path string, r *ExecConfig) (proc.Proce
408412
Terminal: r.Terminal,
409413
},
410414
waitBlock: make(chan struct{}),
411-
pid: &safePid{},
412415
}
413416
e.execState = &execCreatedState{p: e}
414417
return e, nil

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

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

159+
p.pid.Lock()
160+
defer p.pid.Unlock()
161+
159162
var (
160163
err error
161164
socket *runc.Socket
@@ -201,7 +204,7 @@ func (s *createdCheckpointState) Start(ctx context.Context) error {
201204
if err != nil {
202205
return errors.Wrap(err, "failed to retrieve OCI runtime container pid")
203206
}
204-
p.pid = pid
207+
p.pid.pid = pid
205208
return s.transition("running")
206209
}
207210

‎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
@@ -45,6 +45,12 @@ func (s *safePid) get() int {
4545
return s.pid
4646
}
4747

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

0 commit comments

Comments
 (0)