diff --git a/runtime/v1/linux/proc/deleted_state.go b/runtime/v1/linux/proc/deleted_state.go index bc9aefb7f..ab89c3ecb 100644 --- a/runtime/v1/linux/proc/deleted_state.go +++ b/runtime/v1/linux/proc/deleted_state.go @@ -69,7 +69,3 @@ func (s *deletedState) SetExited(status int) { func (s *deletedState) Exec(ctx context.Context, path string, r *ExecConfig) (proc.Process, error) { return nil, errors.Errorf("cannot exec in a deleted state") } - -func (s *deletedState) Pid() int { - return -1 -} diff --git a/runtime/v1/linux/proc/exec.go b/runtime/v1/linux/proc/exec.go index 171ece030..5dbfbaf62 100644 --- a/runtime/v1/linux/proc/exec.go +++ b/runtime/v1/linux/proc/exec.go @@ -49,7 +49,7 @@ type execProcess struct { io runc.IO status int exited time.Time - pid int + pid *safePid closers []io.Closer stdin io.Closer stdio proc.Stdio @@ -69,11 +69,7 @@ func (e *execProcess) ID() string { } func (e *execProcess) Pid() int { - return e.execState.Pid() -} - -func (e *execProcess) pidv() int { - return e.pid + return e.pid.get() } func (e *execProcess) ExitStatus() int { @@ -145,7 +141,7 @@ 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 + pid := e.pid.get() if pid != 0 { if err := unix.Kill(pid, syscall.Signal(sig)); err != nil { return errors.Wrapf(checkKillError(err), "exec kill error") @@ -170,6 +166,12 @@ func (e *execProcess) Start(ctx context.Context) error { } func (e *execProcess) start(ctx context.Context) (err error) { + // The reaper may receive exit signal right after + // the container is started, before the e.pid is updated. + // In that case, we want to block the signal handler to + // access e.pid until it is updated. + e.pid.Lock() + defer e.pid.Unlock() var ( socket *runc.Socket pidfile = filepath.Join(e.path, fmt.Sprintf("%s.pid", e.id)) @@ -229,7 +231,7 @@ func (e *execProcess) start(ctx context.Context) (err error) { if err != nil { return errors.Wrap(err, "failed to retrieve OCI runtime exec pid") } - e.pid = pid + e.pid.pid = pid return nil } @@ -247,11 +249,11 @@ 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 e.pid == 0 { + if e.pid.get() == 0 { return "created", nil } // if we have a pid and it can be signaled, the process is running - if err := unix.Kill(e.pid, 0); err == nil { + if err := unix.Kill(e.pid.get(), 0); err == nil { return "running", nil } // else if we have a pid but it can nolonger be signaled, it has stopped diff --git a/runtime/v1/linux/proc/exec_state.go b/runtime/v1/linux/proc/exec_state.go index 0e0bc3fcf..12489501b 100644 --- a/runtime/v1/linux/proc/exec_state.go +++ b/runtime/v1/linux/proc/exec_state.go @@ -31,7 +31,6 @@ type execState interface { Delete(context.Context) error Kill(context.Context, uint32, bool) error SetExited(int) - Pid() int } type execCreatedState struct { @@ -83,12 +82,6 @@ func (s *execCreatedState) SetExited(status int) { } } -func (s *execCreatedState) Pid() int { - s.p.mu.Lock() - defer s.p.mu.Unlock() - return s.p.pidv() -} - type execRunningState struct { p *execProcess } @@ -127,12 +120,6 @@ func (s *execRunningState) SetExited(status int) { } } -func (s *execRunningState) Pid() int { - s.p.mu.Lock() - defer s.p.mu.Unlock() - return s.p.pidv() -} - type execStoppedState struct { p *execProcess } @@ -170,7 +157,3 @@ func (s *execStoppedState) Kill(ctx context.Context, sig uint32, all bool) error func (s *execStoppedState) SetExited(status int) { // no op } - -func (s *execStoppedState) Pid() int { - return s.p.pidv() -} diff --git a/runtime/v1/linux/proc/init.go b/runtime/v1/linux/proc/init.go index 3e86b34d9..81f6d9514 100644 --- a/runtime/v1/linux/proc/init.go +++ b/runtime/v1/linux/proc/init.go @@ -407,6 +407,7 @@ 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 diff --git a/runtime/v1/linux/proc/utils.go b/runtime/v1/linux/proc/utils.go index 04baae0f7..075ef2617 100644 --- a/runtime/v1/linux/proc/utils.go +++ b/runtime/v1/linux/proc/utils.go @@ -23,6 +23,7 @@ import ( "io" "os" "strings" + "sync" "time" "github.com/containerd/containerd/errdefs" @@ -31,6 +32,18 @@ import ( "golang.org/x/sys/unix" ) +// safePid is a thread safe wrapper for pid. +type safePid struct { + sync.Mutex + pid int +} + +func (s *safePid) get() int { + s.Lock() + defer s.Unlock() + return s.pid +} + // TODO(mlaventure): move to runc package? func getLastRuntimeError(r *runc.Runc) (string, error) { if r.Log == "" {