Skip to content

Commit 4b5265d

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 573d768 commit 4b5265d

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
@@ -19,6 +19,7 @@ import (
1919
"github.com/opencontainers/runc/libcontainer"
2020
"github.com/opencontainers/runc/libcontainer/configs"
2121
"github.com/opencontainers/runc/libcontainer/specconv"
22+
"github.com/opencontainers/runc/libcontainer/system"
2223
"github.com/opencontainers/runc/libcontainer/system/kernelversion"
2324
"github.com/opencontainers/runc/libcontainer/utils"
2425
)
@@ -218,8 +219,11 @@ type runner struct {
218219
}
219220

220221
func (r *runner) run(config *specs.Process) (_ int, retErr error) {
222+
detach := r.detach || (r.action == CT_ACT_CREATE)
221223
defer func() {
222-
if retErr != nil {
224+
// For a non-detached container, or we get an error, we
225+
// should destroy the container.
226+
if !detach || retErr != nil {
223227
r.destroy()
224228
}
225229
}()
@@ -248,11 +252,19 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) {
248252
}
249253
process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i)))
250254
}
251-
detach := r.detach || (r.action == CT_ACT_CREATE)
252255
// Setting up IO is a two stage process. We need to modify process to deal
253256
// with detaching containers, and then we get a tty after the container has
254257
// started.
255-
handlerCh := newSignalHandler(r.enableSubreaper)
258+
if r.enableSubreaper {
259+
// set us as the subreaper before registering the signal handler for the container
260+
if err := system.SetSubreaper(1); err != nil {
261+
logrus.Warn(err)
262+
}
263+
}
264+
var handlerCh chan *signalHandler
265+
if !detach {
266+
handlerCh = newSignalHandler()
267+
}
256268
tty, err := setupIO(process, r.container, config.Terminal, detach, r.consoleSocket)
257269
if err != nil {
258270
return -1, err
@@ -300,15 +312,12 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) {
300312
return -1, err
301313
}
302314
}
303-
handler := <-handlerCh
304-
status, err := handler.forward(process, tty, detach)
305315
if detach {
306316
return 0, nil
307317
}
308-
if err == nil {
309-
r.destroy()
310-
}
311-
return status, err
318+
// For non-detached container, we should forward signals to the container.
319+
handler := <-handlerCh
320+
return handler.forward(process, tty)
312321
}
313322

314323
func (r *runner) destroy() {

0 commit comments

Comments
 (0)