From b74a4c964bcfa807b8ea2c5043614c080f468e1f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 23 Sep 2026 10:18:47 -0400 Subject: [PATCH] fix: the image brings its own PID 1 instead of asking for init: true (4295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator, 2026-09-23: *"it is out of the norm to require this init call we need to fix this."* Correct, and it is the same mistake as declaring the healthcheck per service — the image needing a deployment to remember a flag before it behaves correctly. PID 1 carries a duty no other process has: every orphaned process in the container reparents to it and must be reaped or it stays a zombie holding a PID slot. This app makes orphans in normal operation — six service modules shell out (gallery_dl, thumbnailer, backup_service, external_fetch, download_service, download_backends) and celery's prefork pool forks children that spawn them. Whatever the role, something that is not an init ends up as PID 1: supervisord for `all`, hypercorn for `web`, celery for a worker. `init: true` covered that, and cost correctness the moment it was forgotten or silently dropped — an older Swarm, a plain `docker run`, a compose file someone copied. No signal either way. So tini goes in the image and is the ENTRYPOINT. `docker run ` is correct on its own now, `init: true` comes out of docker-compose.single.yml, and nothing downstream has to know. The smoke asserts /proc/1/comm is tini, read from /proc because the runtime stage installs no `ps`. ## A correction to what I told the operator I justified `init: true` by saying supervisord "has no idea about orphans it never started". That is very likely wrong: supervisord's reaper calls waitpid(-1) and logs "reaped unknown pid" for children it did not spawn, so it does reap orphans. I asserted the mechanism without checking it, and could not check it here — `supervisor` is not installed in this environment. It does not change this commit. tini is correct whichever way that lands, and it covers the single-role containers too, where celery or hypercorn is PID 1 and the subprocess-spawning is heaviest. But the reason I gave was not a verified one and should not have been stated as fact. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- .forgejo/workflows/build.yml | 13 +++++++++++++ Dockerfile | 25 ++++++++++++++++++++++++- docker-compose.single.yml | 6 ------ 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 078c4d0..e01fdc9 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -1553,6 +1553,19 @@ jobs: exit 1 fi echo "smoke: every lane answered" + + # PID 1 is an init, and the IMAGE provides it — no `init: true` in + # whatever runs this. Read from /proc rather than `ps`, which the + # runtime stage does not install. + PID1=$(docker exec "$CID_ALL" cat /proc/1/comm) + echo "smoke: pid 1 is $PID1" + if [ "$PID1" != "tini" ]; then + echo "smoke: FAILED — pid 1 is '$PID1', not an init." >&2 + echo "smoke: orphaned gallery-dl/ffmpeg/pg_dump processes would" >&2 + echo "smoke: accumulate as zombies, and the image would be back" >&2 + echo "smoke: to needing init:true from every deployment." >&2 + exit 1 + fi # Say WHICH processes supervisord is running, so a lane that is # merely restart-looping is visible rather than inferred. # diff --git a/Dockerfile b/Dockerfile index 3cc8e7c..633c34b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,6 +28,10 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ postgresql-client \ zstd \ megatools \ + # PID 1 for every role. See the ENTRYPOINT note at the foot of this file: + # without it the image needs `init: true` in whatever runs it, which is a + # deployment remembering a flag for the image to behave correctly. + tini \ libjpeg62-turbo \ libwebp7 \ libpng16-16 \ @@ -142,7 +146,26 @@ EXPOSE 8080 HEALTHCHECK --interval=30s --timeout=15s --start-period=90s --retries=3 \ CMD ["python", "-m", "backend.app.scripts.healthcheck"] -ENTRYPOINT ["./entrypoint.sh"] +# tini is PID 1, and the image brings its own rather than asking the +# deployment for one. +# +# PID 1 carries a duty no other process has: every orphaned process in the +# container reparents to it and must be reaped, or it stays a zombie holding +# a PID slot. This app makes orphans in normal operation — six service +# modules shell out (gallery-dl, ffmpeg, pg_dump, the external fetchers) and +# celery's prefork pool forks children that spawn them. +# +# Whatever the role, something that is not an init ends up as PID 1: +# supervisord for `all`, hypercorn for `web`, celery for a worker. The fix +# was `init: true` in the compose/stack file, which is out of the norm and +# put correct process handling in the hands of whoever deploys the image — +# the same mistake as declaring the healthcheck per service. A flag that is +# silently dropped (an older Swarm, a `docker run` without it) costs reaping +# with no signal at all. +# +# So the image owns it. `docker run ` is correct on its own, and +# nothing downstream has to know. The smoke asserts /proc/1/comm is tini. +ENTRYPOINT ["/usr/bin/tini", "--", "./entrypoint.sh"] # The DEFAULT is the whole application, not one lane of it. # # `docker run fabledcurator` with no command starts hypercorn plus every diff --git a/docker-compose.single.yml b/docker-compose.single.yml index d112ae7..20a6c23 100644 --- a/docker-compose.single.yml +++ b/docker-compose.single.yml @@ -63,12 +63,6 @@ services: # under supervisord — is what the image does by default, and supervisord's # config is generated from the application's own lane table so the two # cannot disagree. `command: ["all"]` still works and means the same thing. - # tini as PID 1, in front of supervisord. supervisord reaps its own - # children, but a container's PID 1 also inherits orphans from anywhere - # below — celery's prefork pool and gallery-dl's subprocesses both make - # them. Without this they accumulate as zombies for the life of the - # container. - init: true # Sized to the SLOWEST lane, not the average. maintenance_long runs DB # backups, library audits and translation backfill, and gets 180s to # finish a chunk; the lanes stop in parallel, so this covers the max