msys2 / msys2-docker

MSYS2 Docker Images
7 stars 3 forks source link

potential for zombie processes if not running docker with `--init` option #5

Open jeremyd2019 opened 1 week ago

jeremyd2019 commented 1 week ago

In GHA, at least when using tmate action, it seems that PID 1 (serving the role init should) is filled by an instance of tail. Obviously, that does not reap unparented child processes like init is supposed to, and wine appears to use the double-fork trick to avoid having to wait on child processes, which means that PID 1 then has to. The workaround seems to be to specify the --init option to docker: https://docs.github.com/en/actions/using-jobs/running-jobs-in-a-container#setting-container-resource-options https://docs.docker.com/reference/cli/docker/container/create/#options

jobs:
  main:
    runs-on: ubuntu-latest
    container:
      image: ghcr.io/msys2/msys2-docker-experimental:latest
      options: --init
    steps:
      - name: tmate
        uses: mxschmitt/action-tmate@v3

I think this is something that just needs to be documented, I don't know if there's anything that can be done to the image to solve this. Then again, I'm no docker expert...

lazka commented 1 week ago

I think this is something that just needs to be documented, I don't know if there's anything that can be done to the image to solve this. Then again, I'm no docker expert...

I think the usual thing is to install and use "tini" (https://packages.debian.org/bookworm/tini) in the image, which is the equivalent to using "--init".

jeremyd2019 commented 1 week ago

For fun, after running the msys2 script, pacman -Syuu, and exiting, this is the ps output:

root@87830e4af3b5:/__w/docker-wine-test/docker-wine-test# ps ax
    PID TTY      STAT   TIME COMMAND
      1 ?        Ss     0:00 tail -f /dev/null
     16 ?        Ssl    0:00 /__e/node20/bin/node /__w/_actions/mxschmitt/action-tmate/v3/lib/index.js
    220 ?        Zs     0:00 [tmate] <defunct>
    221 ?        S      0:00 /tmp/tmate/tmate -S /tmp/tmate.sock -a /github/home/.ssh/authorized_keys set-option -g default-command bash --rcfile /tmp/tmate.bashrc ; new-session -d
    222 pts/0    Ss     0:00 bash --rcfile /tmp/tmate.bashrc
    337 pts/0    Z      0:00 [Xvfb] <defunct>
    354 pts/0    Z      0:00 [Xvfb] <defunct>
    359 ?        Zs     0:02 [wineserver] <defunct>
    361 pts/0    Z      0:00 [wineboot.exe] <defunct>
    363 ?        Zs     0:00 [services.exe] <defunct>
    367 pts/0    Z      0:00 [conhost.exe] <defunct>
    369 pts/0    Z      0:01 [bash.exe] <defunct>
    373 ?        Zs     0:00 [explorer.exe] <defunct>
    376 pts/0    Z      0:00 [bash.exe] <defunct>
    381 pts/0    Z      0:00 [cygpath.exe] <defunct>
    384 pts/0    Z      0:00 [bash.exe] <defunct>
    387 pts/0    Z      0:00 [bash.exe] <defunct>
    390 pts/0    Z      0:00 [ln.exe] <defunct>
    393 pts/0    Z      0:00 [bash.exe] <defunct>
    396 pts/0    Z      0:00 [id.exe] <defunct>
    399 pts/0    Z      0:00 [bash.exe] <defunct>
    402 pts/0    Z      0:00 [hostname.exe] <defunct>
    405 pts/0    Z      0:00 [bash.exe] <defunct>
    408 pts/0    Z      0:00 [which.exe] <defunct>
    411 pts/0    Z      0:00 [bash.exe] <defunct>
    414 pts/0    Z      0:00 [bash.exe] <defunct>
    417 pts/0    Z      0:00 [uname.exe] <defunct>
    420 pts/0    Z      0:00 [bash.exe] <defunct>
    423 pts/0    Z      0:00 [locale.exe] <defunct>
    426 pts/0    Z      0:00 [bash.exe] <defunct>
    429 pts/0    Z      0:00 [tzset.exe] <defunct>
    432 pts/0    Z      0:00 [bash.exe] <defunct>
    435 pts/0    Z      0:00 [bash.exe] <defunct>
    438 pts/0    Z      0:00 [bash.exe] <defunct>
    441 pts/0    Z      0:00 [bash.exe] <defunct>
    444 pts/0    Z      0:00 [bash.exe] <defunct>
    446 pts/0    Z      0:00 [id.exe] <defunct>
    450 pts/0    Z      0:00 [bash.exe] <defunct>
    454 pts/0    Z      0:00 [bash.exe] <defunct>
    458 pts/0    Z      0:00 [bash.exe] <defunct>
    460 pts/0    Z      0:00 [getent.exe] <defunct>
    464 pts/0    Z      0:00 [cut.exe] <defunct>
    467 pts/0    Z      0:00 [grep.exe] <defunct>
    470 pts/0    Z      0:00 [bash.exe] <defunct>
    473 pts/0    Z      0:00 [bash.exe] <defunct>
    476 pts/0    Z      0:00 [pacman.exe] <defunct>
    485 pts/0    Z      0:00 [bash.exe] <defunct>
    488 pts/0    Z      0:00 [ps.exe] <defunct>
    491 pts/0    Z      0:00 [bash.exe] <defunct>
    510 pts/0    R+     0:00 ps ax

This does not happen when adding the options: --init line to the workflow:

root@6fe4cb9bd82e:/__w/docker-wine-test/docker-wine-test# ps ax
    PID TTY      STAT   TIME COMMAND
      1 ?        Ss     0:00 /sbin/docker-init -- tail -f /dev/null
      7 ?        S      0:00 tail -f /dev/null
     16 ?        Ssl    0:00 /__e/node20/bin/node /__w/_actions/mxschmitt/action
    221 ?        S      0:00 /tmp/tmate/tmate -S /tmp/tmate.sock -a /github/home
    222 pts/0    Ss     0:00 bash --rcfile /tmp/tmate.bashrc
    482 pts/0    R+     0:00 ps ax
lazka commented 1 week ago

Does https://github.com/msys2/msys2-docker/pull/6 help?

jeremyd2019 commented 1 week ago

I think GHA runner overrides the entrypoint: https://github.com/actions/runner/blob/054fc2e0463b4640a0e9fcaae97aea2279bd81b0/src/Runner.Worker/ContainerOperationProvider.cs#L322-L326

jeremyd2019 commented 1 week ago

Specifically, I believe that overriding the entrypoint with tail -f /dev/null is more-or-less specific to jobs which specify a container:, it looks like "container actions" are allowed to specify an entry point.

As such, I don't know how to test a non-deployed image in that specific configuration, but I think if somebody wanted to use the image in GHA they would probably want to use it like that, so they could then specify steps: probably with msys2 as the shell:.

lazka commented 1 week ago

Do you think #6 helps in some cases? Or should I close it?

jeremyd2019 commented 1 week ago

I don't know enough about docker to say one way or the other. It seems like it may help some scenario, but as far as I know it won't help the GHA scenario I encountered, and perhaps the best thing to do for that is just document it and suggest adding options: --init to the workflow

lazka commented 1 week ago

ok, documentation changes welcome ;)