Commit Graph

1 Commits

Author SHA1 Message Date
Zannis Kalampoukis 5db8ce3c44
fix(docker): make tini PID 1 in the server image so adopted orphans are reaped (#12137)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent runs execute inside the server container, and they spawn many
short-lived descendants: git, the adapter CLI, esbuild, sh
> - The server image sets `ENTRYPOINT ["docker-entrypoint.sh"]`, and
that entrypoint ends in `exec`, so node becomes PID 1
> - Node reaps only the children it spawned itself. It installs no
`SIGCHLD`/`waitpid` handler for orphans that the kernel re-parents onto
PID 1, so those orphans stay as zombies forever
> - Zombies accumulate monotonically. When the cgroup pid limit is
reached, every `fork()` in the container fails and the instance is dead
> - This pull request installs `tini` and makes it PID 1 in front of the
existing entrypoint, adds a behavioural test that proves reaping, and
adds a `pids_limit` backstop to both compose files
> - The benefit is that a long-running container no longer degrades into
total fork failure, and a future regression is caught by CI instead of
by an outage

Depends-on: none — this change is self-contained in the image build and
its tests, and it touches no other in-flight branch

## Linked Issues or Issue Description

No public GitHub issue exists for this defect. It was found on a live
long-running instance. Description follows the bug report template.

**What happened?**

The server container ran for 22 hours and reached 2039 of 2048 pids in
its cgroup. Of 1760 processes, 1731 were zombies, and all 1731 had PID 1
as their parent. PID 1 was `node --import
./server/node_modules/tsx/dist/loader.mjs server/dist/index.js`. Zombies
accrued at about 79 per hour and were never reaped. The oldest zombie
was 20.8 hours old against a container uptime of 22.0 hours, so nothing
had been reaped since boot. Once the pid limit was reached, `git` and
`gh` failed with `pthread_create failed: Resource temporarily
unavailable`.

**Expected behavior**

PID 1 reaps orphaned processes that the kernel re-parents onto it. The
pid count of a long-running container stays flat instead of growing
without bound.

**Steps to reproduce**

1. Start the server image without `docker run --init` and without `init:
true`.
2. Run agent work that spawns descendants which outlive their immediate
parent.
3. Read `/sys/fs/cgroup/pids.current` and count processes in `Z` state
over several hours.
4. The zombie count grows monotonically and every zombie has PPID 1.

**Relevant logs or output**

```
cgroup pids.current / pids.max : 2039 / 2048
total processes                : 1760
  zombies                      : 1731  (98.4%)
  parent of every zombie       : PID 1  (1731/1731)
PID 1 cmdline                  : node --import .../tsx/dist/loader.mjs server/dist/index.js
container uptime               : 22.0 h
oldest zombie                  : 20.8 h    median: 14.4 h
zombie names                   : git 717, claude 280, MainThread 167, sleep 141,
                                 esbuild 138, postgres 76, sh 65, sccache 50
```

**Additional context**

The fix pattern is already in this repository.
`docker/agent-runtime/Dockerfile.base` installs `tini` and sets
`ENTRYPOINT ["/usr/bin/tini", "--"]`. It was never applied to the server
image.

## What Changed

- `Dockerfile`: install `tini` in the `base` stage and set `ENTRYPOINT
["/usr/bin/tini", "--", "docker-entrypoint.sh"]`. The entrypoint stays
in the exec chain, so UID/GID remapping, `gosu`, and graceful shutdown
are unchanged.
- `scripts/assert-orphan-reaping.sh` (new): a behavioural probe. It
spawns a leader that forks a grandchild, exits the leader, and asserts
that the orphaned grandchild leaves `Z` state instead of persisting. It
fails closed if the grandchild is not re-parented onto PID 1, so a pass
cannot mean the check ran too early.
- `.github/workflows/docker.yml`: run that probe against the pushed
image after the publish step. The publish step is multi-arch with `push:
true`, so nothing is loaded into the runner daemon and the pushed tag is
the only thing to test. The cloud variant is `FROM production` and
inherits the same `ENTRYPOINT`.
- `scripts/docker-build-test.sh`: run the same probe against a local
build.
- `docker/docker-compose.yml` and
`docker/docker-compose.quickstart.yml`: add `pids_limit: 2048` as a
backstop, so a future leak dies visibly at its own ceiling instead of
starving the host of pids.
- `server/src/__tests__/container-init-reaping.test.ts` (new): 13
assertions that guard the configuration the probe depends on.

No per-orchestrator init lever was added. The image owning PID 1 covers
compose, plain `docker run`, the quadlet units, and the ECS task
definition in one place. Adding `init: true` in compose or
`initProcessEnabled` on the ECS task would nest a second init around
`tini`, and `tini` then warns on every boot that it is not PID 1. The
new test asserts the absence of both levers across all three manifests,
so the decision survives the next edit.

## Verification

| Check | Result |
|---|---|
| `scripts/assert-orphan-reaping.sh` against a real init | Grandchild
re-parented to PPID 1, then reaped. Exit 0. |
| Same probe forced against a genuine zombie | Reports `Z` and fails.
The failure branch is not vacuous. |
| Config guard against the pre-fix files | Exactly the 3 relevant
assertions turn red. |
| Config guard with `tini` removed from `apt-get` but the comments kept
| Red. It checks the install, not a mention of the name. |
| `cd server && npx vitest run
src/__tests__/container-init-reaping.test.ts` | 13 passed |
| `npx tsc --noEmit -p server` | Clean |
| `node scripts/check-docker-deps-stage.mjs` | PASS |
| `node --test scripts/release-verify-workflow.test.mjs` | 8 passed |

Not verified locally: no container runtime is available in the authoring
environment, so the probe has not run against a build of this image. The
new `docker.yml` step runs it against the pushed image on this PR.

## Risks

Low risk, but it is an image and entrypoint change, so it affects
deployments.

- `tini` adds one small package to the `base` stage.
`docker/agent-runtime/Dockerfile.base` already installs it from the same
Debian archive.
- Signal handling changes shape: `tini` receives `SIGTERM` and forwards
it to the entrypoint, which `exec`s node. `tini` forwards signals to its
direct child by default, and the exec chain keeps node as that child, so
graceful shutdown is preserved. A reviewer should confirm this on a real
stop.
- `pids_limit: 2048` is new for compose users. A deployment that
legitimately needs more than 2048 processes would now hit the ceiling.
The measured steady state on a busy instance was under 400.
- If a deployment already passes `--init` or `init: true`, `tini` runs
under another init and prints a warning that it is not PID 1. Reaping
still works because the outer init handles it. The compose files in this
repository do not set `init: true`.

## Model Used

Claude Opus 5 (`claude-opus-5`), extended thinking, with tool use and
code execution in an agent harness.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local issues or links
- [x] My branch name describes the change and contains no internal
ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: zannis <1011451+zannis@users.noreply.github.com>
2026-08-25 09:52:39 -07:00