diff --git a/.gitignore b/.gitignore index 680a874..b73a843 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,6 @@ /*.xcodeproj .swiftpm Package.resolved +IntegrationTests/Fixture/.build +IntegrationTests/Fixture/Sources/Fixture/Generated.swift +IntegrationTests/Fixture/deploy diff --git a/IntegrationTests/Fixture/Package.swift b/IntegrationTests/Fixture/Package.swift new file mode 100644 index 0000000..60d600c --- /dev/null +++ b/IntegrationTests/Fixture/Package.swift @@ -0,0 +1,10 @@ +// swift-tools-version:6.0 +import PackageDescription + +// Stands in for a Saga site so the shutdown tests don't need the real Saga +// dependency graph. Built and driven by ../run-shutdown-tests.sh. +let package = Package( + name: "fixture", + products: [.executable(name: "Fixture", targets: ["Fixture"])], + targets: [.executableTarget(name: "Fixture", path: "Sources/Fixture")] +) diff --git a/IntegrationTests/Fixture/Sources/Fixture/main.swift b/IntegrationTests/Fixture/Sources/Fixture/main.swift new file mode 100644 index 0000000..cc5a915 --- /dev/null +++ b/IntegrationTests/Fixture/Sources/Fixture/main.swift @@ -0,0 +1,15 @@ +import Foundation + +// Writes the config file a real Saga site would write, tells the CLI the first +// render is done, then stays alive the way Saga does while watching for changes. +// FIXTURE_HANG=1 skips the signal, leaving the CLI parked in its startup wait. + +let cwd = FileManager.default.currentDirectoryPath +try? #"{"input":"content","output":"deploy"}"# + .write(toFile: cwd + "/.build/saga-config.json", atomically: true, encoding: .utf8) + +if ProcessInfo.processInfo.environment["FIXTURE_HANG"] == nil { + kill(getppid(), SIGUSR2) +} + +Thread.sleep(forTimeInterval: 600) diff --git a/IntegrationTests/run-shutdown-tests.sh b/IntegrationTests/run-shutdown-tests.sh new file mode 100755 index 0000000..259c69a --- /dev/null +++ b/IntegrationTests/run-shutdown-tests.sh @@ -0,0 +1,114 @@ +#!/bin/bash +# Checks that Ctrl-C during `saga dev` always kills its children. +# +# Child processes get their own process group, so a SIGINT the CLI doesn't +# handle itself reaches only the CLI and leaves children running under launchd. +# Each scenario interrupts at a different point in the CLI's lifecycle and then +# asserts nothing survived. +# +# Usage: IntegrationTests/run-shutdown-tests.sh [port] + +set -u +set -m # job control, so background children don't inherit SIGINT=SIG_IGN + +ROOT=$(cd "$(dirname "$0")/.." && pwd) +FIXTURE=$ROOT/IntegrationTests/Fixture +SAGA=$ROOT/.build/debug/saga +PORT=${1:-3999} +LOG=$(mktemp) + +fail() { echo "error: $*" >&2; exit 1; } + +[ -x "$SAGA" ] || fail "$SAGA not found, run 'swift build' first" + +# A file big enough that rebuilding it takes long enough to interrupt. +mkdir -p "$FIXTURE/Sources/Fixture" "$FIXTURE/content" "$FIXTURE/deploy" +echo "fixture" > "$FIXTURE/deploy/index.html" +python3 -c " +print('// generated by run-shutdown-tests.sh') +for i in range(6000): print(f'func slowFunc{i}(_ x: Int) -> Int {{ x &+ {i} }}') +" > "$FIXTURE/Sources/Fixture/Generated.swift" || fail "could not generate fixture source" + +(cd "$FIXTURE" && swift build) > /dev/null 2>&1 || fail "fixture failed to build" + +reset() { + pkill -f "saga dev --port $PORT" 2>/dev/null + pkill -f "Fixture/.build/debug/Fixture" 2>/dev/null + sleep 0.5 +} + +# $1 = label, $2 = point to interrupt at +scenario() { + local label=$1 when=$2 + reset + cd "$FIXTURE" || exit 1 + if [ "$when" = startup ]; then export FIXTURE_HANG=1; else unset FIXTURE_HANG; fi + [ "$when" = initial-build ] && touch "$FIXTURE/Sources/Fixture/Generated.swift" + + "$SAGA" dev --port "$PORT" > "$LOG" 2>&1 & + local dev=$! site= build= + + case $when in + product-lookup) + for _ in $(seq 1 100); do build=$(pgrep -P $dev -f "dump-package" | head -1); [ -n "$build" ] && break; sleep 0.05; done + ;; + initial-build) + for _ in $(seq 1 200); do build=$(pgrep -P $dev -f "swift-build|swift build" | head -1); [ -n "$build" ] && break; sleep 0.05; done + ;; + startup) + for _ in $(seq 1 200); do site=$(pgrep -P $dev -f "debug/Fixture" | head -1); [ -n "$site" ] && break; sleep 0.1; done + ;; + idle|recompile) + for _ in $(seq 1 300); do + lsof -nP -iTCP:"$PORT" -sTCP:LISTEN -t 2>/dev/null | grep -qx "$dev" && break + sleep 0.1 + done + site=$(pgrep -P $dev -f "debug/Fixture" | head -1) + if [ "$when" = recompile ]; then + touch "$FIXTURE/Sources/Fixture/Generated.swift" + kill -USR1 $dev + for _ in $(seq 1 100); do + build=$(pgrep -P $dev | grep -v "^$site\$" | head -1); [ -n "$build" ] && break; sleep 0.05 + done + fi + ;; + esac + + kill -INT $dev 2>/dev/null + wait $dev 2>/dev/null + sleep 1 + + local bad=0 detail="" + kill -0 $dev 2>/dev/null && { detail="$detail cli-still-running"; bad=1; } + [ -n "$site" ] && kill -0 "$site" 2>/dev/null && { detail="$detail site-orphaned"; bad=1; } + [ -n "$build" ] && kill -0 "$build" 2>/dev/null && { detail="$detail build-orphaned"; bad=1; } + lsof -nP -iTCP:"$PORT" -sTCP:LISTEN -t >/dev/null 2>&1 && { detail="$detail port-still-open"; bad=1; } + grep -q "Shutting down" "$LOG" || { detail="$detail no-shutdown-handler"; bad=1; } + + # An empty pid means the scenario never caught the process it meant to + # interrupt, so a pass would prove nothing. + case $when in + product-lookup|initial-build|recompile) [ -z "$build" ] && { detail="$detail never-observed-build"; bad=1; } ;; + startup|idle) [ -z "$site" ] && { detail="$detail never-observed-site"; bad=1; } ;; + esac + + if [ $bad -eq 0 ]; then + printf " ok %-22s (site=%s build=%s)\n" "$label" "${site:--}" "${build:--}" + else + printf " FAIL %-22s%s\n" "$label" "$detail" + fi + reset + return $bad +} + +echo "Ctrl-C during \`saga dev\` (port $PORT)" +rc=0 +scenario "product lookup" product-lookup || rc=1 +scenario "initial build" initial-build || rc=1 +scenario "startup window" startup || rc=1 +scenario "idle" idle || rc=1 +scenario "recompile" recompile || rc=1 + +rm -f "$LOG" +[ $rc -eq 0 ] && echo "All shutdown tests passed." || echo "Shutdown tests failed." +exit $rc diff --git a/Sources/SagaCLI/DevCommand.swift b/Sources/SagaCLI/DevCommand.swift index b12ae12..ed8dde2 100644 --- a/Sources/SagaCLI/DevCommand.swift +++ b/Sources/SagaCLI/DevCommand.swift @@ -1,6 +1,7 @@ import ArgumentParser import Foundation import SagaPathKit +import os struct Dev: ParsableCommand { static let configuration = CommandConfiguration( @@ -18,76 +19,114 @@ struct Dev: ParsableCommand { } try cachePath.mkpath() - // Find the executable product name from Package.swift - guard let productName = findExecutableProduct() else { - print("Could not find an executable product in Package.swift") - throw ExitCode.failure - } - - // Initial build - log("Building site...") - guard swiftBuild() else { - log("Initial build failed.") - throw ExitCode.failure - } - - let coordinator = DevCoordinator(productName: productName, cachePath: cachePath, port: port) + let coordinator = DevCoordinator(cachePath: cachePath, port: port) try coordinator.start() } } /// Manages the dev server lifecycle: site process, HTTP server, signal handling. private final class DevCoordinator: @unchecked Sendable { - let productName: String let cachePath: Path let port: Int - var siteProcess: Process? var server: DevServer? - init(productName: String, cachePath: Path, port: Int) { - self.productName = productName + /// Resolved during startup, before the recompile handler is installed. + private var productName = "" + + /// State that shutdown and recompile both touch. + private struct Lifecycle { + var siteProcess: Process? + var buildProcess: Process? + var shuttingDown = false + } + + private let lifecycle = OSAllocatedUnfairLock(initialState: Lifecycle()) + + /// Serializes recompiles. Shutdown never uses this, so Ctrl-C doesn't wait on + /// an in-flight build. + private let recompileQueue = DispatchQueue(label: "Saga.Recompile") + + init(cachePath: Path, port: Int) { self.cachePath = cachePath self.port = port } func start() throws { + // Shutdown handling goes up before anything is spawned. Children get their own + // process group, so a Ctrl-C we don't handle ourselves kills only the CLI and + // leaves them running. + signal(SIGINT, SIG_IGN) + let sigintSrc = DispatchSource.makeSignalSource(signal: SIGINT, queue: DispatchQueue(label: "Saga.Shutdown")) + sigintSrc.setEventHandler { [weak self] in + print("\nShutting down...") + guard let self else { Foundation.exit(0) } + self.shutdown() + } + sigintSrc.resume() + // Set up SIGUSR2 handler — Saga signals us when a content rebuild completes so we can reload browsers signal(SIGUSR2, SIG_IGN) let sigusr2Source = DispatchSource.makeSignalSource(signal: SIGUSR2, queue: DispatchQueue(label: "Saga.Signal")) sigusr2Source.setEventHandler { [weak self] in self?.server?.sendReload() } sigusr2Source.resume() - // Set up SIGUSR1 handler — Saga signals us when Swift source files change so we can recompile + // The recompile handler is installed after startup, so a source change now + // can't race the launch below. signal(SIGUSR1, SIG_IGN) - let sigusr1Source = DispatchSource.makeSignalSource(signal: SIGUSR1, queue: DispatchQueue(label: "Saga.Recompile")) - sigusr1Source.setEventHandler { [weak self] in self?.recompileAndRelaunch() } - sigusr1Source.resume() + + // Find the executable product name from Package.swift + guard let productName = findExecutableProduct(onStart: { [weak self] in self?.track($0) }) else { + print("Could not find an executable product in Package.swift") + throw ExitCode.failure + } + self.productName = productName + lifecycle.withLock { $0.buildProcess = nil } + + log("Building site...") + guard build() else { + log("Initial build failed.") + throw ExitCode.failure + } // Launch the site process. Saga watches its own files and rebuilds internally. - siteProcess = launchSiteProcess(productName: productName, cachePath: cachePath) - guard siteProcess != nil else { + // Registering it under the lock keeps shutdown from exiting between the two. + let (siteProcess, alreadyShuttingDown) = lifecycle.withLock { state -> (Process?, Bool) in + guard !state.shuttingDown else { return (nil, true) } + let process = launchSiteProcess(productName: productName, cachePath: cachePath) + state.siteProcess = process + return (process, false) + } + if alreadyShuttingDown { + dispatchMain() // shutdown() is mid-flight and exits the process + } + guard let siteProcess else { log("Failed to launch site process.") throw ExitCode.failure } // Wait for the initial build to complete (SIGUSR2 or process exit) let initialBuild = DispatchSemaphore(value: 0) - siteProcess?.terminationHandler = { _ in initialBuild.signal() } + siteProcess.terminationHandler = { _ in initialBuild.signal() } let initialSigusr2 = DispatchSource.makeSignalSource(signal: SIGUSR2, queue: DispatchQueue(label: "Saga.InitialBuild")) initialSigusr2.setEventHandler { initialBuild.signal() } initialSigusr2.resume() initialBuild.wait() initialSigusr2.cancel() - siteProcess?.terminationHandler = nil + siteProcess.terminationHandler = nil // Read the config file written by Saga to detect output path. // If the config file doesn't exist, this is a Saga 2 site which is not supported. guard let config = readSagaConfig() else { log("This version of saga-cli requires Saga 3.x or later.") - siteProcess?.terminate() + terminate(siteProcess) throw ExitCode.failure } + // Set up SIGUSR1 handler — Saga signals us when Swift source files change so we can recompile + let sigusr1Source = DispatchSource.makeSignalSource(signal: SIGUSR1, queue: recompileQueue) + sigusr1Source.setEventHandler { [weak self] in self?.recompileAndRelaunch() } + sigusr1Source.resume() + // Start the dev server let devServer = DevServer(outputPath: config.output, port: port) server = devServer @@ -109,33 +148,67 @@ private final class DevCoordinator: @unchecked Sendable { // Open the browser openBrowser(url: "http://localhost:\(port)/") - // Handle Ctrl+C shutdown - let sigintSrc = DispatchSource.makeSignalSource(signal: SIGINT, queue: DispatchQueue(label: "Saga.Signals")) - sigintSrc.setEventHandler { [weak self] in - print("\nShutting down...") - self?.siteProcess?.terminate() - self?.server?.stop() - Foundation.exit(0) - } - sigintSrc.resume() - signal(SIGINT, SIG_IGN) - withExtendedLifetime((sigusr1Source, sigusr2Source, sigintSrc)) { dispatchMain() } } + /// Kills whatever is running and exits. Safe to call more than once. + private func shutdown() { + // Set the flag and read the processes in one step, so a recompile can't + // install a replacement we'd leave running. + let processes = lifecycle.withLock { state -> (Process?, Process?)? in + guard !state.shuttingDown else { return nil } + state.shuttingDown = true + return (state.siteProcess, state.buildProcess) + } + guard let (site, build) = processes else { return } + + terminate(build) + terminate(site) + server?.stop() + Foundation.exit(0) + } + + /// Records a helper process so shutdown can interrupt it. Shutdown may have run + /// while the process was starting, in which case it found nothing to kill. + private func track(_ process: Process) { + let alreadyShuttingDown = lifecycle.withLock { state -> Bool in + state.buildProcess = process + return state.shuttingDown + } + if alreadyShuttingDown { + terminate(process) + } + } + + /// Runs `swift build`, tracking the process so shutdown can interrupt it. + private func build() -> Bool { + let built = swiftBuild { [weak self] in self?.track($0) } + lifecycle.withLock { $0.buildProcess = nil } + return built + } + func recompileAndRelaunch() { + guard !lifecycle.withLock({ $0.shuttingDown }) else { return } + log("Source code changed, recompiling...") - guard swiftBuild() else { + let built = build() + guard !lifecycle.withLock({ $0.shuttingDown }) else { return } + + guard built else { log("Build failed, waiting for next change...") return } - // Build succeeded — kill old process and launch new one - siteProcess?.terminate() - siteProcess?.waitUntilExit() + // Shutdown may hold this same process, but terminate() is safe to call twice. + terminate(lifecycle.withLock { $0.siteProcess }) - siteProcess = launchSiteProcess(productName: productName, cachePath: cachePath) + // One step, or shutdown could exit between the check and the launch and + // orphan the new process. + lifecycle.withLock { state in + guard !state.shuttingDown else { return } + state.siteProcess = launchSiteProcess(productName: productName, cachePath: cachePath) + } } } diff --git a/Sources/SagaCLI/Utils.swift b/Sources/SagaCLI/Utils.swift index e745ac1..88379e3 100644 --- a/Sources/SagaCLI/Utils.swift +++ b/Sources/SagaCLI/Utils.swift @@ -26,7 +26,8 @@ func log(_ message: String) { } /// Find the first executable product name using `swift package dump-package`. -func findExecutableProduct() -> String? { +/// `onStart` receives the process, so callers can interrupt it. +func findExecutableProduct(onStart: ((Process) -> Void)? = nil) -> String? { let process = Process() process.executableURL = URL(fileURLWithPath: "/usr/bin/env") process.arguments = ["swift", "package", "dump-package"] @@ -38,6 +39,7 @@ func findExecutableProduct() -> String? { do { try process.run() + onStart?(process) process.waitUntilExit() guard process.terminationStatus == 0 else { return nil } @@ -74,7 +76,8 @@ func findExecutableProduct() -> String? { } } -func swiftBuild() -> Bool { +/// Runs `swift build`. `onStart` receives the process, so callers can interrupt it. +func swiftBuild(onStart: ((Process) -> Void)? = nil) -> Bool { let process = Process() process.executableURL = URL(fileURLWithPath: "/usr/bin/env") process.arguments = ["swift", "build"] @@ -83,6 +86,7 @@ func swiftBuild() -> Bool { do { try process.run() + onStart?(process) process.waitUntilExit() return process.terminationStatus == 0 } catch { @@ -91,6 +95,24 @@ func swiftBuild() -> Bool { } } +/// Terminates `process`, escalating to SIGKILL after `timeout`. Shutdown runs with +/// SIGINT ignored, so waiting forever on a child that blocks SIGTERM would leave the +/// CLI unkillable from its own terminal. +func terminate(_ process: Process?, timeout: TimeInterval = 5) { + guard let process, process.isRunning else { return } + process.terminate() + + let deadline = Date().addingTimeInterval(timeout) + while process.isRunning, Date() < deadline { + Thread.sleep(forTimeInterval: 0.01) + } + + if process.isRunning { + kill(process.processIdentifier, SIGKILL) + process.waitUntilExit() + } +} + func launchSiteProcess(productName: String, cachePath: Path) -> Process? { let binPath = FileManager.default.currentDirectoryPath + "/.build/debug/\(productName)"