Skip to content

Fix Up Command Foreground Interactions when sending signals#127

Merged
Cyb3rDudu merged 4 commits into
Mcrich23:mainfrom
adrum:fix/compose-up-foreground
Jul 8, 2026
Merged

Fix Up Command Foreground Interactions when sending signals#127
Cyb3rDudu merged 4 commits into
Mcrich23:mainfrom
adrum:fix/compose-up-foreground

Conversation

@adrum

@adrum adrum commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

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.

adrum added 2 commits June 19, 2026 21:53
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 Cyb3rDudu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Cyb3rDudu

Copy link
Copy Markdown
Collaborator

#126 modifies WaitForeverCpuTests.swift (the baseline fix); #127 deletes it (replacing waitForever with runForegroundUntilStopped). You need a small rebase on that file.

adrum added 2 commits July 4, 2026 19:10
…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.
@adrum

adrum commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

I added support for the pattern defined in #97 for container name resolution, and I also backported the #126 idle-baseline technique into ForegroundWaitCpuTests.

Let me know if you see anything else!

@adrum adrum requested a review from Cyb3rDudu July 5, 2026 16:45
@Cyb3rDudu Cyb3rDudu merged commit afd708c into Mcrich23:main Jul 8, 2026
1 check passed
Cyb3rDudu added a commit that referenced this pull request Jul 12, 2026
)

`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
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.

2 participants