diff --git a/signals.go b/signals.go index 15f16de75..fc0033caa 100644 --- a/signals.go +++ b/signals.go @@ -5,7 +5,6 @@ import ( "os/signal" "github.com/opencontainers/runc/libcontainer" - "github.com/opencontainers/runc/libcontainer/system" "github.com/opencontainers/runc/libcontainer/utils" "github.com/sirupsen/logrus" @@ -16,13 +15,7 @@ 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(enableSubreaper bool) chan *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) - } - } +func newSignalHandler() chan *signalHandler { handler := make(chan *signalHandler) // signal.Notify is actually quite expensive, as it has to configure the // signal mask and add signal handlers for all signals (all ~65 of them). @@ -54,13 +47,9 @@ type signalHandler struct { // forward handles the main signal event loop forwarding, resizing, or reaping depending // on the signal received. -func (h *signalHandler) forward(process *libcontainer.Process, tty *tty, detach bool) (int, error) { +func (h *signalHandler) forward(process *libcontainer.Process, tty *tty) (int, error) { // make sure we know the pid of our main process so that we can return // after it dies. - if detach { - return 0, nil - } - pid1, err := process.Pid() if err != nil { return -1, err diff --git a/utils_linux.go b/utils_linux.go index a83840bed..6858a7a48 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -20,6 +20,7 @@ import ( "github.com/opencontainers/runc/libcontainer" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/specconv" + "github.com/opencontainers/runc/libcontainer/system" "github.com/opencontainers/runc/libcontainer/system/kernelversion" "github.com/opencontainers/runc/libcontainer/utils" ) @@ -220,8 +221,11 @@ type runner struct { } func (r *runner) run(config *specs.Process) (_ int, retErr error) { + detach := r.detach || (r.action == CT_ACT_CREATE) defer func() { - if retErr != nil { + // For a non-detached container, or we get an error, we + // should destroy the container. + if !detach || retErr != nil { r.destroy() } }() @@ -254,11 +258,19 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) { } process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i))) } - detach := r.detach || (r.action == CT_ACT_CREATE) // Setting up IO is a two stage process. We need to modify process to deal // with detaching containers, and then we get a tty after the container has // started. - handlerCh := newSignalHandler(r.enableSubreaper) + if r.enableSubreaper { + // set us as the subreaper before registering the signal handler for the container + if err := system.SetSubreaper(1); err != nil { + logrus.Warn(err) + } + } + var handlerCh chan *signalHandler + if !detach { + handlerCh = newSignalHandler() + } tty, err := setupIO(process, r.container, config.Terminal, detach, r.consoleSocket) if err != nil { return -1, err @@ -306,15 +318,12 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) { return -1, err } } - handler := <-handlerCh - status, err := handler.forward(process, tty, detach) if detach { return 0, nil } - if err == nil { - r.destroy() - } - return status, err + // For non-detached container, we should forward signals to the container. + handler := <-handlerCh + return handler.forward(process, tty) } func (r *runner) destroy() {