Skip to content

Commit e4266af

Browse files
committed
skip setup signal notifier for detached container
For detached container, we don't need to setup signal notifier, because there is no customer to consume the signals in `forward()`. Signed-off-by: lifubang <lifubang@acmcoder.com>
1 parent 7189de2 commit e4266af

File tree

2 files changed

+20
-22
lines changed

2 files changed

+20
-22
lines changed

signals.go

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"os/signal"
66

77
"github.com/opencontainers/runc/libcontainer"
8-
"github.com/opencontainers/runc/libcontainer/system"
98
"github.com/opencontainers/runc/libcontainer/utils"
109

1110
"github.com/sirupsen/logrus"
@@ -16,13 +15,7 @@ const signalBufferSize = 2048
1615

1716
// newSignalHandler returns a signal handler for processing SIGCHLD and SIGWINCH signals
1817
// while still forwarding all other signals to the process.
19-
func newSignalHandler(enableSubreaper bool) chan *signalHandler {
20-
if enableSubreaper {
21-
// set us as the subreaper before registering the signal handler for the container
22-
if err := system.SetSubreaper(1); err != nil {
23-
logrus.Warn(err)
24-
}
25-
}
18+
func newSignalHandler() chan *signalHandler {
2619
handler := make(chan *signalHandler)
2720
// signal.Notify is actually quite expensive, as it has to configure the
2821
// signal mask and add signal handlers for all signals (all ~65 of them).
@@ -54,13 +47,9 @@ type signalHandler struct {
5447

5548
// forward handles the main signal event loop forwarding, resizing, or reaping depending
5649
// on the signal received.
57-
func (h *signalHandler) forward(process *libcontainer.Process, tty *tty, detach bool) (int, error) {
50+
func (h *signalHandler) forward(process *libcontainer.Process, tty *tty) (int, error) {
5851
// make sure we know the pid of our main process so that we can return
5952
// after it dies.
60-
if detach {
61-
return 0, nil
62-
}
63-
6453
pid1, err := process.Pid()
6554
if err != nil {
6655
return -1, err

utils_linux.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/opencontainers/runc/libcontainer"
1919
"github.com/opencontainers/runc/libcontainer/configs"
2020
"github.com/opencontainers/runc/libcontainer/specconv"
21+
"github.com/opencontainers/runc/libcontainer/system"
2122
"github.com/opencontainers/runc/libcontainer/system/kernelversion"
2223
"github.com/opencontainers/runc/libcontainer/utils"
2324
)
@@ -217,8 +218,11 @@ type runner struct {
217218
}
218219

219220
func (r *runner) run(config *specs.Process) (_ int, retErr error) {
221+
detach := r.detach || (r.action == CT_ACT_CREATE)
220222
defer func() {
221-
if retErr != nil {
223+
// For a non-detached container, or we get an error, we
224+
// should destroy the container.
225+
if !detach || retErr != nil {
222226
r.destroy()
223227
}
224228
}()
@@ -247,11 +251,19 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) {
247251
}
248252
process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i)))
249253
}
250-
detach := r.detach || (r.action == CT_ACT_CREATE)
251254
// Setting up IO is a two stage process. We need to modify process to deal
252255
// with detaching containers, and then we get a tty after the container has
253256
// started.
254-
handlerCh := newSignalHandler(r.enableSubreaper)
257+
if r.enableSubreaper {
258+
// set us as the subreaper before registering the signal handler for the container
259+
if err := system.SetSubreaper(1); err != nil {
260+
logrus.Warn(err)
261+
}
262+
}
263+
var handlerCh chan *signalHandler
264+
if !detach {
265+
handlerCh = newSignalHandler()
266+
}
255267
tty, err := setupIO(process, r.container, config.Terminal, detach, r.consoleSocket)
256268
if err != nil {
257269
return -1, err
@@ -299,15 +311,12 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) {
299311
return -1, err
300312
}
301313
}
302-
handler := <-handlerCh
303-
status, err := handler.forward(process, tty, detach)
304314
if detach {
305315
return 0, nil
306316
}
307-
if err == nil {
308-
r.destroy()
309-
}
310-
return status, err
317+
// For non-detached container, we should forward signals to the container.
318+
handler := <-handlerCh
319+
return handler.forward(process, tty)
311320
}
312321

313322
func (r *runner) destroy() {

0 commit comments

Comments
 (0)