Skip to content

Give a timed-out solver a chance to clean up after itself - #218

Draft
coord-e wants to merge 3 commits into
mainfrom
claude/docker-container-persistence-dddom9
Draft

Give a timed-out solver a chance to clean up after itself#218
coord-e wants to merge 3 commits into
mainfrom
claude/docker-container-persistence-dddom9

Conversation

@coord-e

@coord-e coord-e commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Closes #49.

The cause

Two things together, of which only the first is described in the issue.

Thrust ends a solver that exceeds its timeout with SIGKILL (terminate_for_timeout), which the process cannot act on. But even a wrapper that could run a trap would not have been enough: docker run is only a client of the daemon, so killing it does not touch the container, and --rm removes a container once it stops, which a running solver never does on its own. Observed after killing the wrapper:

9534     1 /bin/bash ./tests/thrust-pcsat-wrapper ...   # reparented to init, still alive
9536  9534 docker start --attach 7e35cdcd...            # the client is alive too

Nothing tells the container to stop, so the solver runs until it finishes — forever on an instance it cannot solve, which is how the issue was hit.

The change

Thrust sends SIGTERM and kills only what has not exited two seconds later, so a solver command that holds resources of its own can release them. terminate_for_timeout is dropped, which is also what keeps the escalation safe: without it the child is neither killed nor reaped on timeout, so its identifier stays allocated and the later SIGKILL cannot reach an unrelated process.

The wrapper removes its container from a SIGTERM trap. It starts the container in the background and waits on it, because bash runs a trap only once the command in the foreground has finished — which would be the very solver the signal is meant to stop. docker create + docker start --attach replaces docker run so the trap has the container's id before the solver starts; it reports the same output and exit status.

nix (signal feature only) is added for kill; libc directly would have been the first unsafe in src/.

Verification

Against the pinned COAR_IMAGE, with THRUST_SOLVER_TIMEOUT_SECS=1 on tests/ui/pass/iterators/annot_range_loop.rs:

  • error: verification error: Timeout(1s), the running container is gone, and the run takes 3.3s — the 1s timeout plus the 2s grace.
  • A stub solver that ignores SIGTERM (trap '' TERM; sleep 300) is killed by the escalation, and Thrust does not hang on it.
  • cargo test: 308 UI tests and 2 doc tests pass, with no container and no stray .smt2 left behind. The stray temp files are incidentally fixed too — the wrapper's EXIT trap could not run under SIGKILL either.

Note

A solver command that ignores SIGTERM still leaves its own children orphaned, since the escalation reaches only the direct child. That is unchanged, and holds for any command that declines to clean up after itself.


Generated by Claude Code

claude added 3 commits August 14, 2026 01:27
Thrust gives up on a solver that exceeds its timeout by killing the wrapper with
SIGKILL, which runs no trap and leaves the container behind: Docker keeps a
container going after the client attached to it is gone, so the solver ran on in
the background for as long as it took, or forever on an instance it cannot
solve.

Removing it has to be left to a process that outlives the wrapper. That process
waits for the write end of a pipe, which the kernel closes however the wrapper
dies, and then force-removes the container. It needs the container's id before
the solver starts, hence `docker create` followed by `docker start --attach`,
which reports the container's output and exit status just as `docker run` did.
Only the wrapper may hold the pipe open; the subshell that starts the container
closes its copy, or the removal would wait for the solver it is meant to stop.

A container is still left behind if the wrapper is killed in the moment between
`docker create` being asked for a container and the removal being set up, but it
is one that never started, and Thrust's timeout does not expire that early.

Closes #49

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qpacd6KyiRsKFexkrgfX6Q
Thrust ended a solver that exceeded its timeout with SIGKILL, a signal the
process cannot act on, and `tests/thrust-pcsat-wrapper` was therefore left no
way to stop the Docker container it had started. Docker keeps a container
running once the client attached to it is gone, so the solver went on in the
background for as long as the instance took, or forever on one it cannot solve.

Send SIGTERM instead, and kill only what has not exited two seconds later, so a
solver command holding resources of its own can release them. The process is
never reaped in between, so the system cannot hand its identifier to an
unrelated process that the SIGKILL would then reach.

The wrapper removes its container from a SIGTERM trap. It starts the container
in the background and waits on it, because bash runs a trap only once the
command in the foreground has finished -- which would be the very solver the
signal is meant to stop.

Closes #49

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qpacd6KyiRsKFexkrgfX6Q
The step-by-step account of `terminate` and of the wrapper's trap restated the
lines they sat above; what is left is why a solver is signalled rather than
killed, why its identifier is safe to signal, and why the container is removed
rather than stopped. The grace period loses its doc comment along the same
lines, matching the other constants in the crate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qpacd6KyiRsKFexkrgfX6Q
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker container process remain to run after the timeout of thrust-pcsat-wrapper

2 participants