From 932dc856f69cd364c59e97ebf911484ef51f20d8 Mon Sep 17 00:00:00 2001 From: Aaron Paterson Date: Sun, 9 Aug 2026 18:45:50 +0000 Subject: [PATCH 1/2] Stop the API server even when it cannot say what it is running Stopping the services waits for the containers to exit first, and asks the API server which of them are still running to do it. Both the wait and the bootout of that server sat in one do block, so a server that could not answer took the bootout with it into the catch: the command reported that it failed to wait for containers, said nothing about the server, and left it running. That is the case where stopping matters most. A server that cannot list its containers is the one process that needs to go, and the command that exists to stop it is the one that gives up. Waiting is best effort now, and the services are stopped either way. Verified: with the API server running, stopping the system leaves no apiserver process, and starting it brings one back. --- Sources/ContainerCommands/System/SystemStop.swift | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/Sources/ContainerCommands/System/SystemStop.swift b/Sources/ContainerCommands/System/SystemStop.swift index 164dd90fd..226211ba7 100644 --- a/Sources/ContainerCommands/System/SystemStop.swift +++ b/Sources/ContainerCommands/System/SystemStop.swift @@ -85,11 +85,22 @@ extension Application { } try await Task.sleep(for: .seconds(1)) } + } catch { + log.warning("failed to wait for all containers", metadata: ["error": "\(error)"]) + } - log.info("stopping service", metadata: ["label": "\(fullLabel)"]) + // Stopping the services is what this command is for, so it + // happens whether or not the containers could be waited for. + // Asking the API server about its containers is asking the + // service being stopped, and it is when that service is in a + // bad way that stopping it matters most: leaving it running + // because it could not answer leaves the one process that + // needed stopping. + log.info("stopping service", metadata: ["label": "\(fullLabel)"]) + do { try ServiceManager.deregister(fullServiceLabel: fullLabel) } catch { - log.warning("failed to wait for all containers", metadata: ["error": "\(error)"]) + log.warning("failed to stop service", metadata: ["label": "\(fullLabel)", "error": "\(error)"]) } } From ef4b354002bd78f2a71bad1781ea312c1e669bfc Mon Sep 17 00:00:00 2001 From: Aaron Paterson Date: Mon, 10 Aug 2026 10:36:09 -0500 Subject: [PATCH 2/2] Stop the services wherever the session that started them left them `register` bootstraps into the domain of whoever called it, so the domain a service sits in belongs to the session that started it. A system brought up from a terminal that no window server owns lands in `user/`, and the same command from a login session lands in `gui/`. Stop derived its domain the same way, from its own session, and the note above the sweep held that the two would agree, since a domain that had changed would have taken XPC with it. XPC reaches across the difference: a login session drives the containers of a system started over ssh perfectly well, because finding a service is a matter of the Mach namespace rather than of the domain it was bootstrapped into. So the mismatch says nothing when it happens. `bootout` is handed a domain that holds nothing, fails with no such process, and the failure was discarded, leaving a stop that reported stopping every service it named while all of them kept running. The start that followed found them alive and left them as they were, so binaries built since were never the ones being run. Deregistering now asks each domain this user's services could be in, its own first, and says whether any of them gave the service up. A service that could not be stopped is logged rather than dropped. --- .../ContainerCommands/System/SystemStop.swift | 51 +++++++++++-------- Sources/ContainerPlugin/ServiceManager.swift | 32 ++++++++++++ 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/Sources/ContainerCommands/System/SystemStop.swift b/Sources/ContainerCommands/System/SystemStop.swift index 226211ba7..929ce046d 100644 --- a/Sources/ContainerCommands/System/SystemStop.swift +++ b/Sources/ContainerCommands/System/SystemStop.swift @@ -49,8 +49,7 @@ extension Application { } ) - let launchdDomainString = try ServiceManager.getDomainString() - let fullLabel = "\(launchdDomainString)/\(prefix)apiserver" + let apiserverLabel = "\(prefix)apiserver" var running = true do { @@ -88,32 +87,44 @@ extension Application { } catch { log.warning("failed to wait for all containers", metadata: ["error": "\(error)"]) } + } - // Stopping the services is what this command is for, so it - // happens whether or not the containers could be waited for. - // Asking the API server about its containers is asking the - // service being stopped, and it is when that service is in a - // bad way that stopping it matters most: leaving it running - // because it could not answer leaves the one process that - // needed stopping. - log.info("stopping service", metadata: ["label": "\(fullLabel)"]) - do { - try ServiceManager.deregister(fullServiceLabel: fullLabel) - } catch { - log.warning("failed to stop service", metadata: ["label": "\(fullLabel)", "error": "\(error)"]) + // Stopping the services is what this command is for, so it + // happens whether or not the containers could be waited for, and + // whether or not the API server answered its health check at all. + // Asking the API server about its containers is asking the + // service being stopped, and it is when that service is in a + // bad way that stopping it matters most: leaving it running + // because it could not answer leaves the one process that + // needed stopping. + log.info("stopping service", metadata: ["label": "\(apiserverLabel)"]) + do { + if try !ServiceManager.deregisterAnyDomain(serviceLabel: apiserverLabel) { + log.warning( + "failed to stop service", + metadata: ["label": "\(apiserverLabel)", "error": "no launchd domain holds it"]) } + } catch { + log.warning("failed to stop service", metadata: ["label": "\(apiserverLabel)", "error": "\(error)"]) } - // Note: The assumption here is that we would have registered the launchd services - // in the same domain as `launchdDomainString`. This is a fairly sane assumption since - // if somehow the launchd domain changed, XPC interactions would not be possible. + // The domain a service was registered in is the one belonging to + // the session that started it, which is not necessarily this one: + // a system brought up from a terminal no window server owns sits + // in `user/`, and this command run from a login session looks + // in `gui/`. Either session can drive the other's services, + // so a mismatch does not announce itself as a failure to reach + // them; it announces itself as a stop that says it stopped + // everything while every service it named is still running. try ServiceManager.enumerate() .filter { $0.hasPrefix(prefix) } - .filter { $0 != fullLabel } - .map { "\(launchdDomainString)/\($0)" } + .filter { $0 != apiserverLabel } .forEach { log.info("stopping service", metadata: ["label": "\($0)"]) - try? ServiceManager.deregister(fullServiceLabel: $0) + let stopped = (try? ServiceManager.deregisterAnyDomain(serviceLabel: $0)) ?? false + if !stopped { + log.warning("failed to stop service", metadata: ["label": "\($0)"]) + } } } } diff --git a/Sources/ContainerPlugin/ServiceManager.swift b/Sources/ContainerPlugin/ServiceManager.swift index 4d2c36246..b9ab06b03 100644 --- a/Sources/ContainerPlugin/ServiceManager.swift +++ b/Sources/ContainerPlugin/ServiceManager.swift @@ -49,6 +49,30 @@ public struct ServiceManager { status = try runLaunchctlCommand(args: ["bootout", label]) } + /// Deregister a service by its bare label, from whichever domain holds it. + /// + /// The domain a service sits in belongs to the session that registered it, + /// not to the session asking about it now: `register` bootstraps into the + /// domain of whoever called it, so a system started from a terminal no + /// window server owns lands in `user/`, and the same command from a + /// login session lands in `gui/`. Either one can drive the other's + /// services afterwards, because reaching them is a matter of the Mach + /// namespace rather than of the domain they were bootstrapped into. + /// + /// Returns whether a domain gave the service up, so that a caller looking + /// in the wrong place is told so rather than left to assume it was heard. + @discardableResult + public static func deregisterAnyDomain(serviceLabel label: String) throws -> Bool { + for domain in try Self.getDomainStrings() { + var status: Int32 = -1 + try Self.deregister(fullServiceLabel: "\(domain)/\(label)", status: &status) + if status == 0 { + return true + } + } + return false + } + /// Restart a service by a launchd label. public static func kickstart(fullServiceLabel label: String) throws { _ = try runLaunchctlCommand(args: ["kickstart", "-k", label]) @@ -121,6 +145,14 @@ public struct ServiceManager { return outputText.trimmingCharacters(in: .whitespacesAndNewlines) } + /// Every launchd domain a service of this user's could be registered in, + /// the caller's own first so that the common case is answered first. + public static func getDomainStrings() throws -> [String] { + let own = try Self.getDomainString() + let userDomains = ["user/\(getuid())", "gui/\(getuid())"] + return [own] + userDomains.filter { $0 != own } + } + public static func getDomainString() throws -> String { let currentSessionType = try getLaunchdSessionType() switch currentSessionType {