Merge pull request #3366 from crosbymichael/exec-pid

Robust pid locking for shim processes

runtime/v1/linux/proc/exec.go

@@ -31,6 +31,7 @@ import (
 	"golang.org/x/sys/unix"
 
 	"github.com/containerd/console"
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/runtime/proc"
 	"github.com/containerd/fifo"
 	runc "github.com/containerd/go-runc"
@@ -49,7 +50,7 @@ type execProcess struct {
 	io      *processIO
 	status  int
 	exited  time.Time
-	pid     *safePid
+	pid     safePid
 	closers []io.Closer
 	stdin   io.Closer
 	stdio   proc.Stdio
@@ -95,6 +96,7 @@ func (e *execProcess) setExited(status int) {
 	e.status = status
 	e.exited = time.Now()
 	e.parent.Platform.ShutdownConsole(context.Background(), e.console)
+	e.pid.set(StoppedPID)
 	close(e.waitBlock)
 }
 
@@ -142,7 +144,12 @@ func (e *execProcess) Kill(ctx context.Context, sig uint32, _ bool) error {
 
 func (e *execProcess) kill(ctx context.Context, sig uint32, _ bool) error {
 	pid := e.pid.get()
-	if pid != 0 {
+	switch {
+	case pid == 0:
+		return errors.Wrap(errdefs.ErrFailedPrecondition, "process not created")
+	case pid < 0:
+		return errors.Wrapf(errdefs.ErrNotFound, "process already finished")
+	default:
 		if err := unix.Kill(pid, syscall.Signal(sig)); err != nil {
 			return errors.Wrapf(checkKillError(err), "exec kill error")
 		}
@@ -254,10 +261,13 @@ func (e *execProcess) Status(ctx context.Context) (string, error) {
 	}
 	e.mu.Lock()
 	defer e.mu.Unlock()
-	// if we don't have a pid then the exec process has just been created
+	// if we don't have a pid(pid=0) then the exec process has just been created
 	if e.pid.get() == 0 {
 		return "created", nil
 	}
+	if e.pid.get() == StoppedPID {
+		return "stopped", nil
+	}
 	// if we have a pid and it can be signaled, the process is running
 	if err := unix.Kill(e.pid.get(), 0); err == nil {
 		return "running", nil

runtime/v1/linux/proc/init.go

@@ -64,7 +64,7 @@ type Init struct {
 	runtime      *runc.Runc
 	status       int
 	exited       time.Time
-	pid          int
+	pid          safePid
 	closers      []io.Closer
 	stdin        io.Closer
 	stdio        proc.Stdio
@@ -113,6 +113,9 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
 		pio     *processIO
 		pidFile = newPidFile(p.Bundle)
 	)
+	p.pid.Lock()
+	defer p.pid.Unlock()
+
 	if r.Terminal {
 		if socket, err = runc.NewTempConsoleSocket(); err != nil {
 			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 {
 	if err != nil {
 		return errors.Wrap(err, "failed to retrieve OCI runtime container pid")
 	}
-	p.pid = pid
+	p.pid.pid = pid
 	return nil
 }
 
@@ -213,7 +216,7 @@ func (p *Init) ID() string {
 
 // Pid of the process
 func (p *Init) Pid() int {
-	return p.pid
+	return p.pid.get()
 }
 
 // ExitStatus of the process
@@ -272,6 +275,7 @@ func (p *Init) setExited(status int) {
 	p.exited = time.Now()
 	p.status = status
 	p.Platform.ShutdownConsole(context.Background(), p.console)
+	p.pid.set(StoppedPID)
 	close(p.waitBlock)
 }
 
@@ -405,7 +409,6 @@ func (p *Init) exec(ctx context.Context, path string, r *ExecConfig) (proc.Proce
 			Terminal: r.Terminal,
 		},
 		waitBlock: make(chan struct{}),
-		pid:       &safePid{},
 	}
 	e.execState = &execCreatedState{p: e}
 	return e, nil

runtime/v1/linux/proc/init_state.go

@@ -143,6 +143,9 @@ func (s *createdCheckpointState) Start(ctx context.Context) error {
 	p := s.p
 	sio := p.stdio
 
+	p.pid.Lock()
+	defer p.pid.Unlock()
+
 	var (
 		err    error
 		socket *runc.Socket
@@ -182,7 +185,7 @@ func (s *createdCheckpointState) Start(ctx context.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "failed to retrieve OCI runtime container pid")
 	}
-	p.pid = pid
+	p.pid.pid = pid
 	return s.transition("running")
 }
 

runtime/v1/linux/proc/process.go

@@ -23,7 +23,11 @@ import (
 )
 
 // RuncRoot is the path to the root runc state directory
-const RuncRoot = "/run/containerd/runc"
+const (
+	RuncRoot = "/run/containerd/runc"
+	// StoppedPID is the pid assigned after a container has run and stopped
+	StoppedPID = -1
+)
 
 func stateName(v interface{}) string {
 	switch v.(type) {

runtime/v1/linux/proc/utils.go

@@ -47,6 +47,12 @@ func (s *safePid) get() int {
 	return s.pid
 }
 
+func (s *safePid) set(pid int) {
+	s.Lock()
+	s.pid = pid
+	s.Unlock()
+}
+
 // TODO(mlaventure): move to runc package?
 func getLastRuntimeError(r *runc.Runc) (string, error) {
 	if r.Log == "" {