From 3791a1f88fef6386341a61f48a25a14b96e285b0 Mon Sep 17 00:00:00 2001 From: Austin Drummond Date: Sun, 7 Jun 2026 14:34:47 -0400 Subject: [PATCH 1/2] fix(up): match docker compose foreground behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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. --- .../Commands/ComposeUp.swift | 124 +++++++++++++++--- .../ForegroundWaitCpuTests.swift | 75 +++++++++++ .../WaitForeverCpuTests.swift | 72 ---------- 3 files changed, 183 insertions(+), 88 deletions(-) create mode 100644 Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift delete mode 100644 Tests/Container-Compose-StaticTests/WaitForeverCpuTests.swift diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index 97a16f96..72011364 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -188,25 +188,117 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { } if !detach { - await waitForever() + await runForegroundUntilStopped(serviceNames: services.map({ $0.serviceName })) } } - func waitForever() async -> Never { - // `AsyncStream(unfolding: () async -> Void?)` ends only when the - // closure returns `nil`. An empty closure returns `()`, which Swift - // auto-wraps as `.some(())` — never `nil` — so the previous - // `for await _ in AsyncStream(unfolding: {})` produced an - // infinite stream of `Void` values with no `await` between them and - // pinned a CPU core at 100% (issue #27). - // - // Suspending on a continuation that is never resumed parks the task - // indefinitely with zero CPU. `withUnsafeContinuation` (rather than - // `withCheckedContinuation`) avoids the runtime's "continuation leaked" - // diagnostic — leaking is the intent here, since the contract is to - // wait until the process is killed. - await withUnsafeContinuation { (_: UnsafeContinuation) in } - fatalError("unreachable") + /// Foreground (`up` without `--detach`) behavior, matching `docker compose up`: + /// - Ctrl-C (SIGINT) / `kill` (SIGTERM) gracefully stops the project's + /// containers, then exits. A second signal forces an immediate exit. + /// - If the containers stop on their own — or via `container compose down` + /// from another shell — `up` returns instead of hanging forever. + func runForegroundUntilStopped(serviceNames: [String]) async -> Never { + let containerNames = projectName.map { project in + serviceNames.map { "\(project)-\($0)" } + } ?? [] + + // Exit once the containers stop by themselves or are stopped externally. + if !containerNames.isEmpty { + Task { + await Self.waitUntilAllContainersStopped(containerNames) + print("\nAll containers have stopped.") + Foundation.exit(0) + } + } + + // Bridge SIGINT/SIGTERM into an async stream. The `ContainerCommands` + // invoked during `up` leave these signals neutered (SIG_IGN) via + // ContainerAPIService's async signal machinery, so a foreground `up` + // previously ignored Ctrl-C. A `DispatchSource` signal source observes + // them regardless of disposition. + let signals = Self.makeSignalStream([SIGINT, SIGTERM]) + var stopping = false + for await _ in signals { + if !stopping { + stopping = true + print("\nGracefully stopping... (press Ctrl+C again to force)") + Task { + await Self.stopContainers(containerNames) + Foundation.exit(0) + } + } else { + print("\nForcing stop.") + Task { + await Self.killContainers(containerNames) + Foundation.exit(130) + } + } + } + Foundation.exit(0) + } + + /// An `AsyncStream` of the given signals, delivered via `DispatchSource` so + /// they're received even after the disposition has been set to `SIG_IGN`. + private static func makeSignalStream(_ signals: [Int32]) -> AsyncStream { + AsyncStream { continuation in + let queue = DispatchQueue(label: "container-compose.signals") + let sources: [DispatchSourceSignal] = signals.map { sig in + // Ignore the default action so the DispatchSource alone handles it. + signal(sig, SIG_IGN) + let source = DispatchSource.makeSignalSource(signal: sig, queue: queue) + source.setEventHandler { continuation.yield(sig) } + source.resume() + return source + } + continuation.onTermination = { _ in sources.forEach { $0.cancel() } } + } + } + + /// Gracefully stops (without removing) the named containers — the + /// `docker compose up` Ctrl-C contract leaves stopped containers in place. + private static func stopContainers(_ containerNames: [String]) async { + let client = ContainerClient() + for name in containerNames { + guard let container = try? await client.get(id: name) else { continue } + print("Stopping container: \(name)") + do { + try await client.stop(id: container.id) + } catch { + print("Error stopping container \(name): \(error)") + } + } + } + + /// Force-stops the named containers with SIGKILL — the second-Ctrl-C + /// contract, matching `docker compose up`'s "press Ctrl+C again to force". + private static func killContainers(_ containerNames: [String]) async { + let client = ContainerClient() + for name in containerNames { + guard let container = try? await client.get(id: name) else { continue } + print("Killing container: \(name)") + try? await client.kill(id: container.id, signal: "SIGKILL") + } + } + + /// Polls until every named container has been observed running at least once + /// and then none remain running (stopped naturally or via `down`). Requiring + /// "seen running first" avoids returning before the containers have started. + private static func waitUntilAllContainersStopped(_ containerNames: [String], interval: TimeInterval = 1.0) async { + let client = ContainerClient() + var seenRunning = Set() + while !Task.isCancelled { + try? await Task.sleep(nanoseconds: UInt64(interval * 1_000_000_000)) + var running = Set() + for name in containerNames { + if let container = try? await client.get(id: name), container.status == .running { + running.insert(name) + } + } + seenRunning.formUnion(running) + if seenRunning.count == containerNames.count && running.isEmpty { + return + } + } } /// Translates Compose's `entrypoint` + `command` into args for `container run`. diff --git a/Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift b/Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift new file mode 100644 index 00000000..31a113ee --- /dev/null +++ b/Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift @@ -0,0 +1,75 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Morris Richman and the Container-Compose project authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Testing +import Foundation +import Darwin +@testable import ContainerComposeCore + +@Suite("Foreground wait CPU usage", .serialized) +struct ForegroundWaitCpuTests { + + /// Returns user-mode CPU time consumed by this process, in microseconds. + private func userCpuMicroseconds() -> Int64 { + var usage = rusage() + getrusage(RUSAGE_SELF, &usage) + return Int64(usage.ru_utime.tv_sec) * 1_000_000 + Int64(usage.ru_utime.tv_usec) + } + + /// Regression for #27: the foreground `up` wait must suspend, not busy-loop. + /// + /// Method: take a `getrusage(RUSAGE_SELF)` snapshot, spawn a child Task that + /// calls `runForegroundUntilStopped` (with no services, so it just awaits the + /// signal stream and starts no container monitor), sleep for a 1s wall-clock + /// window, take a second snapshot, and compare user-CPU consumed. + /// + /// On the original bug (`for await _ in AsyncStream(unfolding: {})`), + /// the child task pinned one core, so over a 1s window it consumes ~1,000,000 + /// µs (one full core). The wait now suspends on a `DispatchSource` signal + /// stream and consumes essentially nothing. + /// + /// `getrusage(RUSAGE_SELF)` is process-wide, so the other (fast, parallel) + /// static suites add CPU noise — but they finish within the first few hundred + /// ms, whereas a busy-loop runs the entire second. The 1s window plus a + /// 400,000 µs threshold (≈0.4 core-seconds) sits well above that transient + /// noise yet well below a full core's worth of spinning, so the test is + /// reliable in the full parallel suite, not just in isolation. + /// + /// Side effect: this test leaks one suspended task per invocation + /// (`runForegroundUntilStopped` is `-> Never` and the suspended task can't be + /// cancelled from outside). The leak is bounded — each leaked task holds only + /// its stack — and is cleaned up when the test process exits. + @Test("foreground wait does not pin a CPU core (regression for #27)") + func foregroundWaitDoesNotPinCpu() async throws { + let composeUp = ComposeUp() + let before = userCpuMicroseconds() + + // Detached so cancellation propagation from the test doesn't reach it + // (it wouldn't matter — the function ignores cancellation by contract — + // but this makes the leak explicit rather than incidental). + Task.detached { + await composeUp.runForegroundUntilStopped(serviceNames: []) + } + + try await Task.sleep(nanoseconds: 1_000_000_000) // 1s + + let after = userCpuMicroseconds() + let consumed = after - before + + #expect(consumed < 400_000, + "foreground wait consumed \(consumed) µs of user CPU in 1s — likely busy-looping (regression for #27)") + } +} diff --git a/Tests/Container-Compose-StaticTests/WaitForeverCpuTests.swift b/Tests/Container-Compose-StaticTests/WaitForeverCpuTests.swift deleted file mode 100644 index ce99641e..00000000 --- a/Tests/Container-Compose-StaticTests/WaitForeverCpuTests.swift +++ /dev/null @@ -1,72 +0,0 @@ -//===----------------------------------------------------------------------===// -// Copyright © 2025 Morris Richman and the Container-Compose project authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -//===----------------------------------------------------------------------===// - -import Testing -import Foundation -import Darwin -@testable import ContainerComposeCore - -@Suite("waitForever CPU usage", .serialized) -struct WaitForeverCpuTests { - - /// Returns user-mode CPU time consumed by this process, in microseconds. - private func userCpuMicroseconds() -> Int64 { - var usage = rusage() - getrusage(RUSAGE_SELF, &usage) - return Int64(usage.ru_utime.tv_sec) * 1_000_000 + Int64(usage.ru_utime.tv_usec) - } - - /// Regression for #27: `waitForever()` must suspend, not busy-loop. - /// - /// Method: take a `getrusage(RUSAGE_SELF)` snapshot, spawn a child Task - /// that calls `waitForever()`, sleep for 200ms wall-clock, take a second - /// snapshot, and compare user-CPU consumed. - /// - /// On the bug (`for await _ in AsyncStream(unfolding: {})`), the - /// child task pins one core, so user CPU consumed during the 200ms window - /// is around 200,000 µs (one full core). With the fix - /// (`withUnsafeContinuation { _ in }`), the child task suspends and - /// consumes essentially nothing. - /// - /// Threshold of 50,000 µs gives ~4× headroom over a noisy CI baseline - /// (test-runner overhead, parallel tasks) while still reliably catching - /// a single core's worth of busy-loop work. - /// - /// Side effect: this test leaks one suspended task per invocation - /// (`waitForever` is `-> Never` and the suspended task can't be cancelled - /// from outside). The leak is bounded — each leaked task holds only its - /// stack — and is cleaned up when the test process exits. - @Test("waitForever does not pin a CPU core (regression for #27)") - func waitForeverDoesNotPinCpu() async throws { - let composeUp = ComposeUp() - let before = userCpuMicroseconds() - - // Detached so cancellation propagation from the test doesn't reach it - // (it wouldn't matter — the function ignores cancellation by contract — - // but this makes the leak explicit rather than incidental). - Task.detached { - await composeUp.waitForever() - } - - try await Task.sleep(nanoseconds: 200_000_000) // 200ms - - let after = userCpuMicroseconds() - let consumed = after - before - - #expect(consumed < 50_000, - "waitForever consumed \(consumed) µs of user CPU in 200ms — likely busy-looping (regression for #27)") - } -} From 0cb010659f3bd225997ed4e17643776ceba98777 Mon Sep 17 00:00:00 2001 From: Austin Drummond Date: Sat, 4 Jul 2026 19:10:41 -0400 Subject: [PATCH 2/2] fix(up): stop resolved container names on ctrl-c, not - MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runForegroundUntilStopped rebuilt names as -, so containers named via explicit container_name or the dotted . DNS convention (#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 #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. --- .../Commands/ComposeUp.swift | 12 ++--- .../ForegroundWaitCpuTests.swift | 46 ++++++++++++------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index 3c9e8c1a..719ff216 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -258,7 +258,7 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { } if !detach { - await runForegroundUntilStopped(serviceNames: services.map({ $0.serviceName })) + await runForegroundUntilStopped(containerNames: services.map({ containerName(for: $0.serviceName) })) } } @@ -267,11 +267,11 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { /// containers, then exits. A second signal forces an immediate exit. /// - If the containers stop on their own — or via `container compose down` /// from another shell — `up` returns instead of hanging forever. - func runForegroundUntilStopped(serviceNames: [String]) async -> Never { - let containerNames = projectName.map { project in - serviceNames.map { "\(project)-\($0)" } - } ?? [] - + /// + /// Takes resolved container names (not service names): a service's container + /// may be named via explicit `container_name` or the dotted + /// `.` DNS convention, not just `-`. + func runForegroundUntilStopped(containerNames: [String]) async -> Never { // Exit once the containers stop by themselves or are stopped externally. if !containerNames.isEmpty { Task { diff --git a/Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift b/Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift index 31a113ee..d2e863a0 100644 --- a/Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift +++ b/Tests/Container-Compose-StaticTests/ForegroundWaitCpuTests.swift @@ -31,22 +31,25 @@ struct ForegroundWaitCpuTests { /// Regression for #27: the foreground `up` wait must suspend, not busy-loop. /// - /// Method: take a `getrusage(RUSAGE_SELF)` snapshot, spawn a child Task that - /// calls `runForegroundUntilStopped` (with no services, so it just awaits the - /// signal stream and starts no container monitor), sleep for a 1s wall-clock - /// window, take a second snapshot, and compare user-CPU consumed. + /// Method: `getrusage(RUSAGE_SELF)` is process-wide, not per-task — it counts + /// every thread in the process, including whatever other tests Swift Testing + /// is running concurrently in this same process (this suite is `.serialized` + /// internally, but that doesn't stop *other* suites from overlapping it). So + /// rather than an absolute threshold, take a baseline measurement over an + /// idle 200ms window immediately before the real one, then assert on the + /// *incremental* cost the foreground-wait task adds on top of that baseline. + /// This cancels out ambient noise from concurrent tests instead of assuming + /// it's near-zero, which stopped holding once the suite grew large enough + /// that something is almost always running during any given 200ms window. /// /// On the original bug (`for await _ in AsyncStream(unfolding: {})`), - /// the child task pinned one core, so over a 1s window it consumes ~1,000,000 - /// µs (one full core). The wait now suspends on a `DispatchSource` signal - /// stream and consumes essentially nothing. + /// the child task pins one core, so it adds roughly 200,000 µs of user CPU + /// on top of baseline during the 200ms window. The wait now suspends on a + /// `DispatchSource` signal stream (with no containers, so it starts no + /// container monitor) and adds essentially nothing. /// - /// `getrusage(RUSAGE_SELF)` is process-wide, so the other (fast, parallel) - /// static suites add CPU noise — but they finish within the first few hundred - /// ms, whereas a busy-loop runs the entire second. The 1s window plus a - /// 400,000 µs threshold (≈0.4 core-seconds) sits well above that transient - /// noise yet well below a full core's worth of spinning, so the test is - /// reliable in the full parallel suite, not just in isolation. + /// Threshold of 50,000 µs of *incremental* cost gives ~4× headroom while + /// still reliably catching a single core's worth of busy-loop work. /// /// Side effect: this test leaks one suspended task per invocation /// (`runForegroundUntilStopped` is `-> Never` and the suspended task can't be @@ -55,21 +58,30 @@ struct ForegroundWaitCpuTests { @Test("foreground wait does not pin a CPU core (regression for #27)") func foregroundWaitDoesNotPinCpu() async throws { let composeUp = ComposeUp() + + // Baseline: ambient CPU consumed by the whole process over an idle + // 200ms window (no foreground-wait task running), to calibrate against + // whatever else is concurrently running in this test process. + let baselineBefore = userCpuMicroseconds() + try await Task.sleep(nanoseconds: 200_000_000) // 200ms + let baselineConsumed = userCpuMicroseconds() - baselineBefore + let before = userCpuMicroseconds() // Detached so cancellation propagation from the test doesn't reach it // (it wouldn't matter — the function ignores cancellation by contract — // but this makes the leak explicit rather than incidental). Task.detached { - await composeUp.runForegroundUntilStopped(serviceNames: []) + await composeUp.runForegroundUntilStopped(containerNames: []) } - try await Task.sleep(nanoseconds: 1_000_000_000) // 1s + try await Task.sleep(nanoseconds: 200_000_000) // 200ms let after = userCpuMicroseconds() let consumed = after - before + let incremental = consumed - baselineConsumed - #expect(consumed < 400_000, - "foreground wait consumed \(consumed) µs of user CPU in 1s — likely busy-looping (regression for #27)") + #expect(incremental < 50_000, + "foreground wait added \(incremental) µs of user CPU over a \(baselineConsumed) µs baseline in 200ms — likely busy-looping (regression for #27)") } }