From 796630258bd8ca4a3d2341a1b504b0356fdbbd06 Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Tue, 2 Jun 2026 14:26:17 -0700 Subject: [PATCH] kill: Wait for container to exit after sigkill (#1589) Today when we send a signal we don't wait for the container to exit, as we don't know what signals the program will actually do anything with. However, sigkill does not fit this mold, and we should wait for the container to exit (or be removed for --rm containers). --- .../Container/ContainerKill.swift | 4 +-- .../Client/ContainerClient.swift | 4 +-- .../Server/Containers/ContainersService.swift | 16 ++++++++-- .../Runtime/RuntimeClient/RuntimeClient.swift | 2 +- .../RuntimeLinux/Server/RuntimeService.swift | 31 +++++++++++++------ 5 files changed, 39 insertions(+), 18 deletions(-) diff --git a/Sources/ContainerCommands/Container/ContainerKill.swift b/Sources/ContainerCommands/Container/ContainerKill.swift index ef95a6d1..b223b9ff 100644 --- a/Sources/ContainerCommands/Container/ContainerKill.swift +++ b/Sources/ContainerCommands/Container/ContainerKill.swift @@ -61,12 +61,10 @@ extension Application { containers = containerIds } - let signalNumber = try Signal(signal).rawValue - var errors: [any Error] = [] for container in containers { do { - try await client.kill(id: container, signal: signalNumber) + try await client.kill(id: container, signal: signal) print(container) } catch { errors.append(error) diff --git a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift index 291e8cf2..b1b64a66 100644 --- a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift +++ b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift @@ -158,12 +158,12 @@ public struct ContainerClient: Sendable { } /// Send a signal to the container. - public func kill(id: String, signal: Int32) async throws { + public func kill(id: String, signal: String) async throws { do { let request = XPCMessage(route: .containerKill) request.set(key: .id, value: id) request.set(key: .processIdentifier, value: id) - request.set(key: .signal, value: Int64(signal)) + request.set(key: .signal, value: signal) try await xpcClient.send(request) } catch { diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index 82726850..834b31bb 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -547,7 +547,7 @@ public actor ContainersService { } /// Send a signal to the container. - public func kill(id: String, processID: String, signal: Int64) async throws { + public func kill(id: String, processID: String, signal: String) async throws { log.debug( "ContainersService: enter", metadata: [ @@ -571,6 +571,13 @@ public actor ContainersService { let state = try self._getContainerState(id: id) let client = try state.getClient() try await client.kill(processID, signal: signal) + + // SIGKILL is guaranteed to terminate the target. When directed at the + // container's init process, follow up with the same API-server cleanup + // that `stop` performs. + if processID == id, (try? Signal(signal)) == .kill { + try await handleContainerExit(id: id) + } } /// Stop all containers inside the sandbox, aborting any processes currently @@ -1123,8 +1130,11 @@ public actor ContainersService { } extension XPCMessage { - func signal() throws -> Int64 { - self.int64(key: .signal) + func signal() throws -> String { + guard let signal = self.string(key: .signal) else { + throw ContainerizationError(.invalidArgument, message: "missing signal in xpc message") + } + return signal } func stopOptions() throws -> ContainerStopOptions { diff --git a/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift b/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift index 3040902d..087a8120 100644 --- a/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift +++ b/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift @@ -195,7 +195,7 @@ extension RuntimeClient { } } - public func kill(_ id: String, signal: Int64) async throws { + public func kill(_ id: String, signal: String) async throws { let request = XPCMessage(route: RuntimeRoutes.kill.rawValue) request.set(key: RuntimeKeys.id.rawValue, value: id) request.set(key: RuntimeKeys.signal.rawValue, value: signal) diff --git a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift index a84fd21d..b320d581 100644 --- a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift +++ b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift @@ -554,11 +554,13 @@ public actor RuntimeService { self.log.debug("enter", metadata: ["func": "\(#function)"]) defer { self.log.debug("exit", metadata: ["func": "\(#function)"]) } - return try await self.lock.withLock { [self] _ in + let id = try message.id() + let signal = try Signal(message.signal()) + + try await self.lock.withLock { [self] _ in switch await self.state { case .running: let ctr = try await getContainer() - let id = try message.id() if id != ctr.container.id { guard let processInfo = await self.processes[id] else { throw ContainerizationError(.invalidState, message: "process \(id) does not exist") @@ -567,13 +569,11 @@ public actor RuntimeService { guard let proc = processInfo.process else { throw ContainerizationError(.invalidState, message: "process \(id) not started") } - try await proc.kill(Signal(rawValue: Int32(try message.signal()))) - return message.reply() + try await proc.kill(signal) + return } - // TODO: fix underlying signal value to int64 - try await ctr.container.kill(Signal(rawValue: Int32(try message.signal()))) - return message.reply() + try await ctr.container.kill(signal) default: throw ContainerizationError( .invalidState, @@ -581,6 +581,16 @@ public actor RuntimeService { ) } } + + // SIGKILL is guaranteed by the kernel to terminate the target, so block + // until we observe the exit. + if signal == .kill { + _ = await withCheckedContinuation { cc in + self.waitForExit(id: id, cont: cc) + } + } + + return message.reply() } /// Resize the terminal for a process. @@ -1252,8 +1262,11 @@ public actor RuntimeService { } extension XPCMessage { - fileprivate func signal() throws -> Int64 { - self.int64(key: RuntimeKeys.signal.rawValue) + fileprivate func signal() throws -> String { + guard let signal = self.string(key: RuntimeKeys.signal.rawValue) else { + throw ContainerizationError(.invalidArgument, message: "missing signal in xpc message") + } + return signal } fileprivate func stopOptions() throws -> ContainerStopOptions {