Interrupt signal is not proxied and "docker run" terminates

[enool]

Description

Interrupt signal (SIGINT) is not proxied to the container when issued with
kill() to "docker run". Run command is terminated by the signal and container
is left running in the background.

This issue is seen on Linux when running a container attached with --tty (-t)
option and INT signal is sent from an external process with kill(). It's not
about CTRL+c INT that does indeed work as expected.

Embedding "docker run" in to software like editors is a bit tricky because of
this. For example, emacs compilation uses a tty by default, and it also
sends INT kill() instead of CTRL+c to the process group on "kill-compilation".

Pull request for moby/term: moby/term#31

Reproduce

docker run -it --rm --sig-proxy --init alpine sleep 3600
pkill --signal INT -f docker.*run.*sleep

Docker run dies and container is left behind

Expected behavior

Signal is proxied from docker run and container dies

docker version

Client:
 Version:           20.10.17
 API version:       1.41
 Go version:        go1.19.1
 Git commit:        100c70180f
 Built:             Mon Sep 26 21:49:37 2022
 OS/Arch:           linux/amd64
 Context:           default
 Experimental:      true

Server:
 Engine:
  Version:          20.10.17
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.19.1
  Git commit:       a89b84221c
  Built:            Sat Oct  1 23:04:37 2022
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.6.8
  GitCommit:        9cd3357b7fd7218e4aec3eae239db1f68a5a6ec6
 runc:
  Version:          1.1.3
  GitCommit:        6724737f999df9ee0d8ca5c6d7b81f97adc34374
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad007797e0dcd8b7126f27bb87401d224240

docker info

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc., v0.9.1)

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 1
 Server Version: 20.10.17
 Storage Driver: fuse-overlayfs
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runtime.v1.linux runc io.containerd.runc.v2
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 9cd3357b7fd7218e4aec3eae239db1f68a5a6ec6
 runc version: 6724737f999df9ee0d8ca5c6d7b81f97adc34374
 init version: de40ad007797e0dcd8b7126f27bb87401d224240
 Security Options:
  apparmor
  seccomp
   Profile: default
 Kernel Version: 5.19.5-gentoo
 Operating System: Gentoo Linux
 OSType: linux
 Architecture: x86_64
 CPUs: 24
 Total Memory: 31.29GiB
 Name: deski
 ID: AMHJ:VETK:BOAC:BVC3:RTZ2:LQJA:4H4H:IHA3:U3ZQ:23GQ:XNKE:YFCA
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No cpu cfs quota support
WARNING: No cpu cfs period support
WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support

Additional Info

No response

[thaJeztah]

I was confused for a bit, because I recalled we made a change in 20.10 to fix this (#1841), but I see this is specifically about SIGINT.

Let me ask around if there's possibly a reason for excluding this specifically 🤔

[enool]

Sure, thanks!

In case you missed, moby/term#31 has some more information.

[enool]

Seems like signal handlers for INT on docker/cli and moby/term are also racing.

Adding a sleep before exiting changes the behaviour as INT signal is then proxied correctly:

@@ -10,6 +11,7 @@ import (
        "io"
        "os"
        "os/signal"
+       "time"

        "golang.org/x/sys/unix"
 )
@@ -112,6 +114,7 @@ func handleInterrupt(fd uintptr, state *State) {
                        // quit cleanly and the new terminal item is on a new line
                        fmt.Println()
                        signal.Stop(sigchan)
+                       time.Sleep(time.Second * 3)
                        close(sigchan)
                        RestoreTerminal(fd, state)
                        os.Exit(1)`

[enool]

Here is some ramble on why I think it would make sense to fix this:

1. Current behavior is contradicting what is documented in docker-run man page for sig-proxy. It mentions explicitly what signals are not proxied:

--sig-proxy=true|false Proxy received signals to the process (non-TTY mode only). SIGCHLD, SIGSTOP, and SIGKILL are not proxied. The default is true.

2. Current behavior is not well defined or reliable. Two different goroutines are racing to deal with SIGINT. Anything that would depend on this could have flaky outcome. This was shown in the comment above with a sleep instrumentation.

3. Removing handleInterrupt() in moby/term only affects direct kill(SIGINT,...) and would not change interactive Ctrl+C behavior of --tty=true nor --tty=false.

   In case of --tty=false, signaling is not changed as there is no raw terminal setup that would add the troublesome handleInterrupt(). With --sig-proxy enabled, this is simply a forwarded signal.

   With --tty=true, troublesome handleInterrupt() is still not involved in Ctrl+C. Raw terminal mode is set up with line discipline -SIG tty setting. This prevents kernel from generating a SIGINT to the foreground processes. A Ctrl+C is simply forwarded as \x03 end-of-text character on the hijacked stream with --tty.

I've got a vague feeling I forgot to mention something 🤷. Well, it would be kewl to have this.

[thaJeztah]

Sorry for not getting back to you. I did ask around if people knew if there was some reason they could come up with why the handleInterrupt() was there, but no real ideas (as far as I understood).

In those discussions, I suggested to at least try a "test" PR to see if everything works as intended; I just opened #3849, which uses the version of github.com/moby/term from your PR (moby/term#31)

[enool]

No worries! Happy to hear back from you.

Thanks again for getting this forward.