From 87a429088f17910b1a75cd2110bcc72eebbf5b5b Mon Sep 17 00:00:00 2001 From: Raj Date: Mon, 3 Nov 2025 17:36:26 -0800 Subject: [PATCH 1/2] Kill sandbox service on exit to prevent restart race condition --- .../Containers/ContainersService.swift | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/Sources/Services/ContainerAPIService/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Containers/ContainersService.swift index 8f1b7694e..ae22f7a78 100644 --- a/Sources/Services/ContainerAPIService/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Containers/ContainersService.swift @@ -223,6 +223,32 @@ public actor ContainersService { return } + let path = self.containerRoot.appendingPathComponent(id) + let bundle = ContainerClient.Bundle(path: path) + let config = try bundle.configuration + let label = Self.fullLaunchdServiceLabel( + runtimeName: config.runtimeHandler, + instanceId: id + ) + + let isRegistered = try ServiceManager.isRegistered(fullServiceLabel: label) + + if isRegistered { + // Stale service exists + self.log.warning("Found stale launchd service for \(id), cleaning up") + try? ServiceManager.kill(fullServiceLabel: label, signal: SIGKILL) + try? await Task.sleep(for: .milliseconds(100)) + try? ServiceManager.deregister(fullServiceLabel: label) + } + + // Always register service + try Self.registerService( + plugin: self.runtimePlugins.first { $0.name == config.runtimeHandler }!, + loader: self.pluginLoader, + configuration: config, + path: path + ) + let runtime = state.snapshot.configuration.runtimeHandler let sandboxClient = try await SandboxClient.create( id: id, @@ -457,14 +483,27 @@ public actor ContainersService { await self.exitMonitor.stopTracking(id: id) - // Try and shutdown the runtime helper. + // Shutdown the runtime helper immediately to prevent reconnection do { self.log.info("Shutting down sandbox service for \(id)") + let path = self.containerRoot.appendingPathComponent(id) + let bundle = ContainerClient.Bundle(path: path) + let config = try bundle.configuration + let label = Self.fullLaunchdServiceLabel( + runtimeName: config.runtimeHandler, + instanceId: id + ) + let client = try state.getClient() - try await client.shutdown() + try? await client.shutdown() + try? ServiceManager.kill(fullServiceLabel: label, signal: SIGKILL) + try? await Task.sleep(for: .milliseconds(100)) + + try ServiceManager.deregister(fullServiceLabel: label) + self.log.info("Cleaned up sandbox service for \(id)") } catch { - self.log.error("failed to shutdown sandbox service for \(id): \(error)") + self.log.error("failed to cleanup sandbox service for \(id): \(error)") } state.snapshot.status = .stopped From c8a4cb79b6ceb44e88c9588a4285b6f905f2e700 Mon Sep 17 00:00:00 2001 From: Raj Date: Tue, 4 Nov 2025 14:05:04 -0800 Subject: [PATCH 2/2] Remove shutdown sleep, let launchd erminate process --- .../Containers/ContainersService.swift | 70 +++++-------------- .../SandboxService.swift | 9 --- 2 files changed, 16 insertions(+), 63 deletions(-) diff --git a/Sources/Services/ContainerAPIService/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Containers/ContainersService.swift index ae22f7a78..143d74590 100644 --- a/Sources/Services/ContainerAPIService/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Containers/ContainersService.swift @@ -90,19 +90,12 @@ public actor ContainersService { ) ) results[config.id] = state - let plugin = runtimePlugins.first { $0.name == config.runtimeHandler } - guard let plugin else { + guard runtimePlugins.first(where: { $0.name == config.runtimeHandler }) != nil else { throw ContainerizationError( .internalError, message: "failed to find runtime plugin \(config.runtimeHandler)" ) } - try Self.registerService( - plugin: plugin, - loader: loader, - configuration: config, - path: dir - ) } catch { try? FileManager.default.removeItem(at: dir) log.warning("failed to load container bundle at \(dir.path)") @@ -159,10 +152,7 @@ public actor ContainersService { ) } - let runtimePlugin = self.runtimePlugins.filter { - $0.name == configuration.runtimeHandler - }.first - guard let runtimePlugin else { + guard self.runtimePlugins.first(where: { $0.name == configuration.runtimeHandler }) != nil else { throw ContainerizationError( .notFound, message: "unable to locate runtime plugin \(configuration.runtimeHandler)" @@ -185,13 +175,6 @@ public actor ContainersService { try bundle.setContainerRootFs(cloning: imageFs) try bundle.write(filename: "options.json", value: options) - try Self.registerService( - plugin: runtimePlugin, - loader: self.pluginLoader, - configuration: configuration, - path: path - ) - let snapshot = ContainerSnapshot( configuration: configuration, status: .stopped, @@ -226,22 +209,6 @@ public actor ContainersService { let path = self.containerRoot.appendingPathComponent(id) let bundle = ContainerClient.Bundle(path: path) let config = try bundle.configuration - let label = Self.fullLaunchdServiceLabel( - runtimeName: config.runtimeHandler, - instanceId: id - ) - - let isRegistered = try ServiceManager.isRegistered(fullServiceLabel: label) - - if isRegistered { - // Stale service exists - self.log.warning("Found stale launchd service for \(id), cleaning up") - try? ServiceManager.kill(fullServiceLabel: label, signal: SIGKILL) - try? await Task.sleep(for: .milliseconds(100)) - try? ServiceManager.deregister(fullServiceLabel: label) - } - - // Always register service try Self.registerService( plugin: self.runtimePlugins.first { $0.name == config.runtimeHandler }!, loader: self.pluginLoader, @@ -483,28 +450,23 @@ public actor ContainersService { await self.exitMonitor.stopTracking(id: id) - // Shutdown the runtime helper immediately to prevent reconnection - do { - self.log.info("Shutting down sandbox service for \(id)") + // Shutdown and deregister the sandbox service + self.log.info("Shutting down sandbox service for \(id)") - let path = self.containerRoot.appendingPathComponent(id) - let bundle = ContainerClient.Bundle(path: path) - let config = try bundle.configuration - let label = Self.fullLaunchdServiceLabel( - runtimeName: config.runtimeHandler, - instanceId: id - ) + let path = self.containerRoot.appendingPathComponent(id) + let bundle = ContainerClient.Bundle(path: path) + let config = try bundle.configuration + let label = Self.fullLaunchdServiceLabel( + runtimeName: config.runtimeHandler, + instanceId: id + ) - let client = try state.getClient() - try? await client.shutdown() - try? ServiceManager.kill(fullServiceLabel: label, signal: SIGKILL) - try? await Task.sleep(for: .milliseconds(100)) + let client = try state.getClient() + try await client.shutdown() - try ServiceManager.deregister(fullServiceLabel: label) - self.log.info("Cleaned up sandbox service for \(id)") - } catch { - self.log.error("failed to cleanup sandbox service for \(id): \(error)") - } + // Deregister the service, launchd will terminate the process + try ServiceManager.deregister(fullServiceLabel: label) + self.log.info("Deregistered sandbox service for \(id)") state.snapshot.status = .stopped state.snapshot.networks = [] diff --git a/Sources/Services/ContainerSandboxService/SandboxService.swift b/Sources/Services/ContainerSandboxService/SandboxService.swift index 2fa46c8f8..9b20a69c8 100644 --- a/Sources/Services/ContainerSandboxService/SandboxService.swift +++ b/Sources/Services/ContainerSandboxService/SandboxService.swift @@ -272,15 +272,6 @@ public actor SandboxService { case .created, .stopped(_), .stopping: await self.setState(.shuttingDown) - Task { - do { - try await Task.sleep(for: .seconds(5)) - } catch { - self.log.error("failed to sleep before shutting down SandboxService: \(error)") - } - self.log.info("Shutting down SandboxService") - exit(0) - } default: throw ContainerizationError( .invalidState,