Merge pull request #646 from crosbymichael/pid-host-block

Destroy container along with processes before stdio

exec.go

@@ -76,6 +76,10 @@ following will output a list of processes running in the container:
 			Value: &cli.StringSlice{},
 			Usage: "add a capability to the bounding set for the process",
 		},
+		cli.BoolFlag{
+			Name:  "no-subreaper",
+			Usage: "disable the use of the subreaper used to reap reparented processes",
+		},
 	},
 	Action: func(context *cli.Context) {
 		if os.Geteuid() != 0 {
@@ -104,7 +108,15 @@ func execProcess(context *cli.Context) (int, error) {
 	if err != nil {
 		return -1, err
 	}
-	return runProcess(container, p, nil, context.String("console"), context.String("pid-file"), detach)
+	r := &runner{
+		enableSubreaper: !context.Bool("no-subreaper"),
+		shouldDestroy:   false,
+		container:       container,
+		console:         context.String("console"),
+		detach:          detach,
+		pidFile:         context.String("pid-file"),
+	}
+	return r.run(p)
 }
 
 func getProcess(context *cli.Context, bundle string) (*specs.Process, error) {

libcontainer/init_linux.go

@@ -346,13 +346,16 @@ func killCgroupProcesses(m cgroups.Manager) error {
 		return err
 	}
 	for _, pid := range pids {
-		if p, err := os.FindProcess(pid); err == nil {
+		p, err := os.FindProcess(pid)
+		if err != nil {
+			logrus.Warn(err)
+			continue
+		}
 		procs = append(procs, p)
 		if err := p.Kill(); err != nil {
 			logrus.Warn(err)
 		}
 	}
-	}
 	if err := m.Freeze(configs.Thawed); err != nil {
 		logrus.Warn(err)
 	}

libcontainer/system/linux.go

@@ -11,6 +11,19 @@ import (
 	"unsafe"
 )
 
+// If arg2 is nonzero, set the "child subreaper" attribute of the
+// calling process; if arg2 is zero, unset the attribute.  When a
+// process is marked as a child subreaper, all of the children
+// that it creates, and their descendants, will be marked as
+// having a subreaper.  In effect, a subreaper fulfills the role
+// of init(1) for its descendant processes.  Upon termination of
+// a process that is orphaned (i.e., its immediate parent has
+// already terminated) and marked as having a subreaper, the
+// nearest still living ancestor subreaper will receive a SIGCHLD
+// signal and be able to wait(2) on the process to discover its
+// termination status.
+const PR_SET_CHILD_SUBREAPER = 36
+
 type ParentDeathSignal int
 
 func (p ParentDeathSignal) Restore() error {
@@ -113,6 +126,11 @@ func RunningInUserNS() bool {
 	return true
 }
 
+// SetSubreaper sets the value i as the subreaper setting for the calling process
+func SetSubreaper(i int) error {
+	return Prctl(PR_SET_CHILD_SUBREAPER, uintptr(i), 0, 0, 0)
+}
+
 func Prctl(option int, arg2, arg3, arg4, arg5 uintptr) (err error) {
 	_, _, e1 := syscall.Syscall6(syscall.SYS_PRCTL, uintptr(option), arg2, arg3, arg4, arg5, 0)
 	if e1 != 0 {

restore.go

@@ -68,6 +68,10 @@ using the runc checkpoint command.`,
 			Value: "",
 			Usage: "specify the file to write the process id to",
 		},
+		cli.BoolFlag{
+			Name:  "no-subreaper",
+			Usage: "disable the use of the subreaper used to reap reparented processes",
+		},
 	},
 	Action: func(context *cli.Context) {
 		imagePath := context.String("image-path")
@@ -140,8 +144,7 @@ func restoreContainer(context *cli.Context, spec *specs.Spec, config *configs.Co
 		return -1, err
 	}
 	defer tty.Close()
-	handler := newSignalHandler(tty)
-	defer handler.Close()
+	handler := newSignalHandler(tty, !context.Bool("no-subreaper"))
 	if err := container.Restore(process, options); err != nil {
 		return -1, err
 	}

signals.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/opencontainers/runc/libcontainer"
+	"github.com/opencontainers/runc/libcontainer/system"
 	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
@@ -16,7 +17,13 @@ const signalBufferSize = 2048
 
 // newSignalHandler returns a signal handler for processing SIGCHLD and SIGWINCH signals
 // while still forwarding all other signals to the process.
-func newSignalHandler(tty *tty) *signalHandler {
+func newSignalHandler(tty *tty, enableSubreaper bool) *signalHandler {
+	if enableSubreaper {
+		// set us as the subreaper before registering the signal handler for the container
+		if err := system.SetSubreaper(1); err != nil {
+			logrus.Warn(err)
+		}
+	}
 	// ensure that we have a large buffer size so that we do not miss any signals
 	// incase we are not processing them fast enough.
 	s := make(chan os.Signal, signalBufferSize)
@@ -107,10 +114,3 @@ func (h *signalHandler) reap() (exits []exit, err error) {
 		})
 	}
 }
-
-func (h *signalHandler) Close() error {
-	if h.tty != nil {
-		return h.tty.Close()
-	}
-	return nil
-}

start.go

@@ -43,6 +43,10 @@ is a directory with a specification file and a root filesystem.`,
 			Value: "",
 			Usage: "specify the file to write the process id to",
 		},
+		cli.BoolFlag{
+			Name:  "no-subreaper",
+			Usage: "disable the use of the subreaper used to reap reparented processes",
+		},
 	},
 	Action: func(context *cli.Context) {
 		bundle := context.String("bundle")
@@ -102,24 +106,20 @@ func startContainer(context *cli.Context, spec *specs.Spec) (int, error) {
 	if err != nil {
 		return -1, err
 	}
-
-	// ensure that the container is always removed if we were the process
-	// that created it.
 	detach := context.Bool("detach")
-	if !detach {
-		defer destroy(container)
-	}
-
 	// Support on-demand socket activation by passing file descriptors into the container init process.
 	listenFDs := []*os.File{}
 	if os.Getenv("LISTEN_FDS") != "" {
 		listenFDs = activation.Files(false)
 	}
-
-	status, err := runProcess(container, &spec.Process, listenFDs, context.String("console"), context.String("pid-file"), detach)
-	if err != nil {
-		destroy(container)
-		return -1, err
+	r := &runner{
+		enableSubreaper: !context.Bool("no-subreaper"),
+		shouldDestroy:   true,
+		container:       container,
+		listenFDs:       listenFDs,
+		console:         context.String("console"),
+		detach:          detach,
+		pidFile:         context.String("pid-file"),
 	}
-	return status, nil
+	return r.run(&spec.Process)
 }

utils.go

@@ -303,49 +303,78 @@ func createContainer(context *cli.Context, id string, spec *specs.Spec) (libcont
 	return factory.Create(id, config)
 }
 
-// runProcess will create a new process in the specified container
-// by executing the process specified in the 'config'.
-func runProcess(container libcontainer.Container, config *specs.Process, listenFDs []*os.File, console string, pidFile string, detach bool) (int, error) {
+type runner struct {
+	enableSubreaper bool
+	shouldDestroy   bool
+	detach          bool
+	listenFDs       []*os.File
+	pidFile         string
+	console         string
+	container       libcontainer.Container
+}
+
+func (r *runner) run(config *specs.Process) (int, error) {
 	process, err := newProcess(*config)
 	if err != nil {
+		r.destroy()
 		return -1, err
 	}
-
-	// Add extra file descriptors if needed
-	if len(listenFDs) > 0 {
-		process.Env = append(process.Env, fmt.Sprintf("LISTEN_FDS=%d", len(listenFDs)), "LISTEN_PID=1")
-		process.ExtraFiles = append(process.ExtraFiles, listenFDs...)
+	if len(r.listenFDs) > 0 {
+		process.Env = append(process.Env, fmt.Sprintf("LISTEN_FDS=%d", len(r.listenFDs)), "LISTEN_PID=1")
+		process.ExtraFiles = append(process.ExtraFiles, r.listenFDs...)
 	}
-
-	rootuid, err := container.Config().HostUID()
+	rootuid, err := r.container.Config().HostUID()
 	if err != nil {
+		r.destroy()
 		return -1, err
 	}
-
-	tty, err := setupIO(process, rootuid, console, config.Terminal, detach)
+	tty, err := setupIO(process, rootuid, r.console, config.Terminal, r.detach)
 	if err != nil {
+		r.destroy()
 		return -1, err
 	}
-	defer tty.Close()
-	handler := newSignalHandler(tty)
-	defer handler.Close()
-	if err := container.Start(process); err != nil {
+	handler := newSignalHandler(tty, r.enableSubreaper)
+	if err := r.container.Start(process); err != nil {
+		r.destroy()
+		tty.Close()
 		return -1, err
 	}
 	if err := tty.ClosePostStart(); err != nil {
+		r.terminate(process)
+		r.destroy()
+		tty.Close()
 		return -1, err
 	}
-	if pidFile != "" {
-		if err := createPidFile(pidFile, process); err != nil {
-			process.Signal(syscall.SIGKILL)
-			process.Wait()
+	if r.pidFile != "" {
+		if err := createPidFile(r.pidFile, process); err != nil {
+			r.terminate(process)
+			r.destroy()
+			tty.Close()
 			return -1, err
 		}
 	}
-	if detach {
+	if r.detach {
+		tty.Close()
 		return 0, nil
 	}
-	return handler.forward(process)
+	status, err := handler.forward(process)
+	if err != nil {
+		r.terminate(process)
+	}
+	r.destroy()
+	tty.Close()
+	return status, err
+}
+
+func (r *runner) destroy() {
+	if r.shouldDestroy {
+		destroy(r.container)
+	}
+}
+
+func (r *runner) terminate(p *libcontainer.Process) {
+	p.Signal(syscall.SIGKILL)
+	p.Wait()
 }
 
 func validateProcessSpec(spec *specs.Process) error {