Fix Up Command Foreground Interactions when sending signals#127
Conversation
Foreground `up` (without --detach) parked in waitForever() with no signal
handling and never noticed when its containers stopped. Combined with the
ContainerCommands signal machinery leaving SIGINT/SIGTERM neutered (SIG_IGN)
after image-pull/network/build calls return, Ctrl-C did nothing and the
process hung even after `container compose down` from another shell.
Replace waitForever() with runForegroundUntilStopped(), matching docker compose:
- SIGINT/SIGTERM (observed via a DispatchSource stream, so they fire despite
the SIG_IGN disposition) gracefully stop the project's containers, then exit
- a second signal force-kills the containers with SIGKILL, then exits
("press Ctrl+C again to force")
- a monitor task exits `up` once all services have run and then stopped —
whether they exit on their own or are stopped via `container compose down`
from another shell (it waits until each container is seen running first, so
it never returns before startup completes)
Also retarget the Mcrich23#27 busy-loop regression test at the new wait function and
make it reliable in the full parallel suite: getrusage(RUSAGE_SELF) is
process-wide, so a 1s window (vs 200ms) lets a real busy-loop's full-core
~1,000,000 µs dominate the transient CPU noise from other parallel suites.
Cyb3rDudu
left a comment
There was a problem hiding this comment.
nice one, the dispatchsource signal handling is the right way around the
SIG_IGN that containerapiservice leaves set, and the two-tap graceful→force
- auto-exit-when-stopped monitor are great.
one change before merging: runForegroundUntilStopped rebuilds names as
<project>-<service>, so it misses explicit container_name and the dotted
<service>.<dnsDomain> names from #97. for those, stopContainers /
killContainers / the monitor do client.get(id: "<project>-<service>") →
nil → skip, so ctrl-c exits container-compose but leaves the containers
running. pass the resolved names instead of service keys —
services.map { containerName(for: $0.serviceName) } — or call
containerName(for:) inside.
minor (non-blocking): the monitor needs to see each container running once
before it'll return, so a fast one-shot that exits before the monitor starts
never satisfies it. the signal path still exits fine.
…to fix/compose-up-foreground # Conflicts: # Tests/Container-Compose-StaticTests/WaitForeverCpuTests.swift
…ice> runForegroundUntilStopped rebuilt names as <project>-<service>, so containers named via explicit container_name or the dotted <service>.<dnsDomain> DNS convention (Mcrich23#97) were never found by the stop/kill/monitor paths — ctrl-c exited container-compose but left those containers running. Pass names resolved through containerName(for:) instead of rebuilding them from service keys. Also port the Mcrich23#126 idle-baseline technique into ForegroundWaitCpuTests (successor of the deleted WaitForeverCpuTests) so the process-wide getrusage flake fixed there isn't reintroduced: assert on incremental CPU over an immediately-preceding idle window rather than an absolute threshold.
) `container-compose up` without `-d` started the first container, then exited 1 with "Error: no runtime client exists: container is stopped", leaving nothing running. Detached `up -d` was unaffected. Root cause: in foreground mode the attached `container run` subprocess runs in a background Task while `waitUntilServiceStarted` polls concurrently. Apple Container reports a container as `.stopped` throughout the image/kernel fetch and only flips to `.running` once init starts, so an early poll caught that pre-running `.stopped`, treated it as a completed one-shot, and threw via `containerWait` (which needs the runtime client, gone once stopped). Fixes: - Track the attached `container run` subprocess with a `ForegroundRunHandle`. In `waitUntilServiceStarted`, only treat `.stopped` as terminal once that subprocess has actually exited; while it is alive, keep polling for `.running`. Detached mode (handle is nil) keeps its existing behaviour. - Seed `waitUntilAllContainersStopped`'s `seenRunning` with all container names. It runs only after `configService` has already confirmed every container started, so a `.stopped` container there is one that exited, not one that hasn't started. Without this, a fast one-shot that exits before the first poll would never be seen `.running` and foreground `up` would hang. This also makes #127's signal handling reachable: previously `configService` crashed before `runForegroundUntilStopped` could install the SIGINT/SIGTERM handler, so Ctrl-C never gracefully stopped the project. Fixes #131
When hitting ctrl+c while running the up command in the foreground, I noticed unexpected (to me) behavior. I was expecting it to behave like docker compose.
This allows for proper signal listening. SIGINT (first ctrl+c) gracefully kills the stack. SIGKILL (second ctrl+c) force kills the stack.