diff --git a/notify_socket.go b/notify_socket.go index afebf4e77e4..728f41e312d 100644 --- a/notify_socket.go +++ b/notify_socket.go @@ -151,6 +151,22 @@ func (s *notifySocket) run(pid1 int) error { } } +// forward reads systemd notifications from the container and forwards them +// to notifySocketHost. +func (s *notifySocket) forward(process *libcontainer.Process, detach bool) error { + if detach { + pid, err := process.Pid() + if err != nil { + return err + } + _ = s.run(pid) + } else { + _ = s.run(os.Getpid()) + go func() { _ = s.run(0) }() + } + return nil +} + // notifyHost tells the host (usually systemd) that the container reported READY. // Also sends MAINPID and BARRIER. func notifyHost(client *net.UnixConn, ready []byte, pid1 int) error { diff --git a/signals.go b/signals.go index 936d751f61f..fc0033caa25 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,15 +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. -// If notifySocket is present, use it to read systemd notifications from the container and -// forward them to notifySocketHost. -func newSignalHandler(enableSubreaper bool, notifySocket *notifySocket) 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). @@ -37,8 +28,7 @@ func newSignalHandler(enableSubreaper bool, notifySocket *notifySocket) chan *si // handle all signals for the process. signal.Notify(s) handler <- &signalHandler{ - signals: s, - notifySocket: notifySocket, + signals: s, } }() return handler @@ -52,33 +42,19 @@ type exit struct { } type signalHandler struct { - signals chan os.Signal - notifySocket *notifySocket + signals chan os.Signal } // 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 && h.notifySocket == nil { - return 0, nil - } - pid1, err := process.Pid() if err != nil { return -1, err } - if h.notifySocket != nil { - if detach { - _ = h.notifySocket.run(pid1) - return 0, nil - } - _ = h.notifySocket.run(os.Getpid()) - go func() { _ = h.notifySocket.run(0) }() - } - // Perform the initial tty resize. Always ignore errors resizing because // stdout might have disappeared (due to races with when SIGHUP is sent). _ = tty.resize() diff --git a/utils_linux.go b/utils_linux.go index bd45bf05937..2477408969b 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -19,6 +19,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" ) @@ -218,14 +219,16 @@ type runner struct { subCgroupPaths map[string]string } -func (r *runner) run(config *specs.Process) (int, error) { - var err error +func (r *runner) run(config *specs.Process) (_ int, retErr error) { + detach := r.detach || (r.action == CT_ACT_CREATE) defer func() { - if err != nil { + // For a non-detached container, or we get an error, we + // should destroy the container. + if !detach || retErr != nil { r.destroy() } }() - if err = r.checkTerminal(config); err != nil { + if err := r.checkTerminal(config); err != nil { return -1, err } process, err := newProcess(config) @@ -250,11 +253,19 @@ func (r *runner) run(config *specs.Process) (int, 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, r.notifySocket) + 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 @@ -282,29 +293,32 @@ func (r *runner) run(config *specs.Process) (int, error) { if err != nil { return -1, err } + defer func() { + // We should terminate the process once we got an error. + if retErr != nil { + r.terminate(process) + } + }() if err = tty.waitConsole(); err != nil { - r.terminate(process) return -1, err } tty.ClosePostStart() if r.pidFile != "" { if err = createPidFile(r.pidFile, process); err != nil { - r.terminate(process) return -1, err } } - handler := <-handlerCh - status, err := handler.forward(process, tty, detach) - if err != nil { - r.terminate(process) + if r.notifySocket != nil { + if err = r.notifySocket.forward(process, detach); err != nil { + return -1, err + } } 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() {