diff --git a/exec.go b/exec.go index 0f7fe881..85ba352f 100644 --- a/exec.go +++ b/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) { diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 24e8f714..eb8ad83a 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -346,11 +346,14 @@ func killCgroupProcesses(m cgroups.Manager) error { return err } for _, pid := range pids { - if p, err := os.FindProcess(pid); err == nil { - procs = append(procs, p) - if err := p.Kill(); err != nil { - logrus.Warn(err) - } + 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 { diff --git a/libcontainer/system/linux.go b/libcontainer/system/linux.go index babf5504..d109d7f8 100644 --- a/libcontainer/system/linux.go +++ b/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 { diff --git a/restore.go b/restore.go index 6883d917..873540d2 100644 --- a/restore.go +++ b/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 } diff --git a/signals.go b/signals.go index f4ea61e8..5eee44e7 100644 --- a/signals.go +++ b/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 -} diff --git a/start.go b/start.go index e8b4f5cb..17a595fc 100644 --- a/start.go +++ b/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") @@ -100,24 +104,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) } diff --git a/utils.go b/utils.go index 3b43e853..45600ef1 100644 --- a/utils.go +++ b/utils.go @@ -321,47 +321,76 @@ 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() }