Skip to content

Commit 66e13e5

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 <[email protected]>
1 parent 47248b9 commit 66e13e5

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 destory the container.
225+
if !detach || retErr != nil {
222226
r.destroy()
223227
}
224228
}()
@@ -255,11 +259,19 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) {
255259
}
256260
process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i)))
257261
}
258-
detach := r.detach || (r.action == CT_ACT_CREATE)
259262
// Setting up IO is a two stage process. We need to modify process to deal
260263
// with detaching containers, and then we get a tty after the container has
261264
// started.
262-
handlerCh := newSignalHandler(r.enableSubreaper)
265+
if r.enableSubreaper {
266+
// set us as the subreaper before registering the signal handler for the container
267+
if err := system.SetSubreaper(1); err != nil {
268+
logrus.Warn(err)
269+
}
270+
}
271+
var handlerCh chan *signalHandler
272+
if !detach {
273+
handlerCh = newSignalHandler()
274+
}
263275
tty, err := setupIO(process, r.container, config.Terminal, detach, r.consoleSocket)
264276
if err != nil {
265277
return -1, err
@@ -302,15 +314,12 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) {
302314
return -1, err
303315
}
304316
}
305-
handler := <-handlerCh
306-
status, err := handler.forward(process, tty, detach)
307317
if detach {
308318
return 0, nil
309319
}
310-
if err == nil {
311-
r.destroy()
312-
}
313-
return status, err
320+
// For non-detached container, we should forward signals to the container.
321+
handler := <-handlerCh
322+
return handler.forward(process, tty)
314323
}
315324

316325
func (r *runner) destroy() {

0 commit comments

Comments
 (0)