Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skip setup signal notifier for detached container #4661

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions signals.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,23 @@ const signalBufferSize = 2048
// 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 {
func newSignalHandler(enableSubreaper bool, detach 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)
}
}
handler := make(chan *signalHandler)
handler := make(chan *signalHandler, 1)
// For detached container, we don't need to setup signal notifier, because
// there is no customer to consume the signals in `forward()`.
if detach {
handler <- &signalHandler{
signals: nil,
notifySocket: notifySocket,
}
return handler
}
// 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).
// So, defer this to a background thread while doing the rest of the io/tty
Expand Down Expand Up @@ -61,20 +70,19 @@ type signalHandler struct {
func (h *signalHandler) forward(process *libcontainer.Process, tty *tty, detach bool) (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 {
if detach {
if h.notifySocket != nil {
_ = h.notifySocket.run(pid1)
return 0, nil
}
return 0, nil
}

if h.notifySocket != nil {
_ = h.notifySocket.run(os.Getpid())
go func() { _ = h.notifySocket.run(0) }()
}
Expand Down
2 changes: 1 addition & 1 deletion utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (r *runner) run(config *specs.Process) (int, error) {
// 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)
handlerCh := newSignalHandler(r.enableSubreaper, detach, r.notifySocket)
tty, err := setupIO(process, r.container, config.Terminal, detach, r.consoleSocket)
if err != nil {
return -1, err
Expand Down