From 847690583f6aa5583617317d12bff50ccbfef96a Mon Sep 17 00:00:00 2001
From: Michael Crosby <crosbymichael@gmail.com>
Date: Wed, 27 Apr 2016 12:00:29 -0700
Subject: [PATCH] Remove sigchld reaper from containerd process

Because we are launching alot of different runc commands to do
operations there is a race between doing a `cmd.Wait()` and getting the
sigchld and reaping it.  We can remove the sigchild reaper from
containerd as long as we make sure we reap the shim process if we are
the parent, i.e. not restored.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
---
 containerd/main.go       |  9 +--------
 runtime/container.go     |  4 ++++
 runtime/process.go       | 11 +++++++++++
 supervisor/sort_test.go  |  4 ++++
 supervisor/supervisor.go |  1 +
 5 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/containerd/main.go b/containerd/main.go
index 635c089..531dfe5 100644
--- a/containerd/main.go
+++ b/containerd/main.go
@@ -134,10 +134,7 @@ func daemon(context *cli.Context) error {
 	// setup a standard reaper so that we don't leave any zombies if we are still alive
 	// this is just good practice because we are spawning new processes
 	s := make(chan os.Signal, 2048)
-	signal.Notify(s, syscall.SIGCHLD, syscall.SIGTERM, syscall.SIGINT)
-	if err := osutils.SetSubreaper(1); err != nil {
-		logrus.WithField("error", err).Error("containerd: set subpreaper")
-	}
+	signal.Notify(s, syscall.SIGTERM, syscall.SIGINT)
 	sv, err := supervisor.New(
 		context.String("state-dir"),
 		context.String("runtime"),
@@ -169,10 +166,6 @@ func daemon(context *cli.Context) error {
 	}
 	for ss := range s {
 		switch ss {
-		case syscall.SIGCHLD:
-			if _, err := osutils.Reap(); err != nil {
-				logrus.WithField("error", err).Warn("containerd: reap child processes")
-			}
 		default:
 			logrus.Infof("stopping containerd after receiving %s", ss)
 			server.Stop()
diff --git a/runtime/container.go b/runtime/container.go
index e935cd0..aced304 100644
--- a/runtime/container.go
+++ b/runtime/container.go
@@ -478,6 +478,7 @@ func (c *container) Exec(pid string, pspec specs.ProcessSpec, s Stdio) (pp Proce
 }
 
 func (c *container) startCmd(pid string, cmd *exec.Cmd, p *process) error {
+	p.cmd = cmd
 	if err := cmd.Start(); err != nil {
 		if exErr, ok := err.(*exec.Error); ok {
 			if exErr.Err == exec.ErrNotFound || exErr.Err == os.ErrNotExist {
@@ -699,6 +700,9 @@ func (c *container) waitForStart(p *process, cmd *exec.Cmd) error {
 
 // isAlive checks if the shim that launched the container is still alive
 func isAlive(cmd *exec.Cmd) (bool, error) {
+	if _, err := syscall.Wait4(cmd.Process.Pid, nil, syscall.WNOHANG, nil); err == nil {
+		return true, nil
+	}
 	if err := syscall.Kill(cmd.Process.Pid, 0); err != nil {
 		if err == syscall.ESRCH {
 			return false, nil
diff --git a/runtime/process.go b/runtime/process.go
index 4b40b21..a4bedde 100644
--- a/runtime/process.go
+++ b/runtime/process.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strconv"
 	"syscall"
@@ -39,6 +40,8 @@ type Process interface {
 	SystemPid() int
 	// State returns if the process is running or not
 	State() State
+	// Wait reaps the shim process if avaliable
+	Wait()
 }
 
 type processConfig struct {
@@ -139,6 +142,7 @@ type process struct {
 	container   *container
 	spec        specs.ProcessSpec
 	stdio       Stdio
+	cmd         *exec.Cmd
 }
 
 func (p *process) ID() string {
@@ -219,6 +223,13 @@ func (p *process) getPidFromFile() (int, error) {
 	return i, nil
 }
 
+// Wait will reap the shim process
+func (p *process) Wait() {
+	if p.cmd != nil {
+		p.cmd.Wait()
+	}
+}
+
 func getExitPipe(path string) (*os.File, error) {
 	if err := syscall.Mkfifo(path, 0755); err != nil && !os.IsExist(err) {
 		return nil, err
diff --git a/supervisor/sort_test.go b/supervisor/sort_test.go
index 2fd8680..406523b 100644
--- a/supervisor/sort_test.go
+++ b/supervisor/sort_test.go
@@ -61,6 +61,10 @@ func (p *testProcess) State() runtime.State {
 	return runtime.Running
 }
 
+func (p *testProcess) Wait() {
+
+}
+
 func TestSortProcesses(t *testing.T) {
 	p := []runtime.Process{
 		&testProcess{"ls"},
diff --git a/supervisor/supervisor.go b/supervisor/supervisor.go
index ba80632..012eef2 100644
--- a/supervisor/supervisor.go
+++ b/supervisor/supervisor.go
@@ -274,6 +274,7 @@ func (s *Supervisor) SendTask(evt Task) {
 
 func (s *Supervisor) exitHandler() {
 	for p := range s.monitor.Exits() {
+		p.Wait()
 		e := &ExitTask{
 			Process: p,
 		}