From 6a3622a571afd259fc6d64d09a9674d0d2aa9c72 Mon Sep 17 00:00:00 2001 From: Bhavesh Varma <151822885+radheradhe01@users.noreply.github.com> Date: Sun, 28 Jun 2026 16:49:05 +0530 Subject: [PATCH 1/3] fix(xpc): adopt upstream cancellable request handling Import apple/container#1862 unchanged on top of current Apple main, preserving the original author and implementation. --- Sources/ContainerXPC/XPCClient.swift | 18 ++++--- Sources/ContainerXPC/XPCReplyBox.swift | 68 ++++++++++++++++++++++++++ Sources/ContainerXPC/XPCServer.swift | 10 ++++ 3 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 Sources/ContainerXPC/XPCReplyBox.swift diff --git a/Sources/ContainerXPC/XPCClient.swift b/Sources/ContainerXPC/XPCClient.swift index bab008509..ea4edb7e5 100644 --- a/Sources/ContainerXPC/XPCClient.swift +++ b/Sources/ContainerXPC/XPCClient.swift @@ -111,15 +111,19 @@ extension XPCClient { } group.addTask { - try await withCheckedThrowingContinuation { cont in - xpc_connection_send_message_with_reply(self.connection, message.underlying, nil) { reply in - do { - let message = try self.parseReply(reply) - cont.resume(returning: message) - } catch { - cont.resume(throwing: error) + let box = XPCReplyBox() + return try await withTaskCancellationHandler { + try await withCheckedThrowingContinuation { cont in + box.store(cont) + xpc_connection_send_message_with_reply(self.connection, message.underlying, nil) { reply in + box.resume { try self.parseReply(reply) } } } + } onCancel: { + // On timeout the group cancels this task. The XPC reply + // handler is not cancellation-aware, so resume the + // continuation here; a late reply becomes a no-op. + box.resume { throw CancellationError() } } } diff --git a/Sources/ContainerXPC/XPCReplyBox.swift b/Sources/ContainerXPC/XPCReplyBox.swift new file mode 100644 index 000000000..24a9a1c07 --- /dev/null +++ b/Sources/ContainerXPC/XPCReplyBox.swift @@ -0,0 +1,68 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// 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. +//===----------------------------------------------------------------------===// + +#if os(macOS) +import Foundation + +/// Resume-once bridge between a `CheckedContinuation` and the XPC reply handler. +/// +/// The XPC reply handler is not cancellation-aware, so a request can outlive a +/// client-side timeout. This box lets either the reply handler or the task's +/// cancellation handler resume the continuation, whichever fires first, while +/// guaranteeing it is resumed exactly once. A continuation resumed twice traps. +final class XPCReplyBox: @unchecked Sendable { + private let lock = NSLock() + private var cont: CheckedContinuation? + private var resumed = false + private var pending: Result? + + /// Store the continuation. If a resume already raced ahead (cancellation + /// before the continuation was installed), honor it immediately. + func store(_ cont: CheckedContinuation) { + lock.lock() + if let pending, !resumed { + resumed = true + lock.unlock() + cont.resume(with: pending) + return + } + self.cont = cont + lock.unlock() + } + + /// Resume the continuation with the result of `body`, at most once. A later + /// call is a no-op, so the reply handler and the cancellation handler can + /// both call it safely. + func resume(_ body: () throws -> XPCMessage) { + let result = Result { try body() } + lock.lock() + guard !resumed else { + lock.unlock() + return + } + guard let cont else { + pending = result + lock.unlock() + return + } + resumed = true + self.cont = nil + lock.unlock() + cont.resume(with: result) + } +} + +#endif diff --git a/Sources/ContainerXPC/XPCServer.swift b/Sources/ContainerXPC/XPCServer.swift index b33453142..a46dc0873 100644 --- a/Sources/ContainerXPC/XPCServer.swift +++ b/Sources/ContainerXPC/XPCServer.swift @@ -241,6 +241,16 @@ public struct XPCServer: Sendable { } xpc_connection_send_message(connection, reply.underlying) } + } else { + // No handler for this route: reply with an error instead of dropping + // the message, otherwise the client blocks until its timeout (or + // forever, if it sent none). + log.error("no handler registered for route", metadata: ["route": "\(route)"]) + Self.replyWithError( + connection: connection, + object: object, + err: ContainerizationError(.invalidArgument, message: "unknown route: \(route)") + ) } } From 0c4f2a11e01caaf5d7f8d135605c421fa0ca5c36 Mon Sep 17 00:00:00 2001 From: Steve Date: Sat, 11 Jul 2026 18:08:09 +0100 Subject: [PATCH 2/3] test(xpc): cover upstream cancellation handling Exercise the timeout, caller-cancellation, late-reply, client-reuse, and unknown-route paths introduced by apple/container#1862. --- Package.swift | 7 + Tests/ContainerXPCTests/XPCClientTests.swift | 263 +++++++++++++++++++ 2 files changed, 270 insertions(+) create mode 100644 Tests/ContainerXPCTests/XPCClientTests.swift diff --git a/Package.swift b/Package.swift index b00e58894..81086f9b2 100644 --- a/Package.swift +++ b/Package.swift @@ -478,6 +478,13 @@ let package = Package( "CAuditToken", ] ), + .testTarget( + name: "ContainerXPCTests", + dependencies: [ + .product(name: "Logging", package: "swift-log"), + "ContainerXPC", + ] + ), .target( name: "ContainerOS", dependencies: [ diff --git a/Tests/ContainerXPCTests/XPCClientTests.swift b/Tests/ContainerXPCTests/XPCClientTests.swift new file mode 100644 index 000000000..324046be5 --- /dev/null +++ b/Tests/ContainerXPCTests/XPCClientTests.swift @@ -0,0 +1,263 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// 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. +//===----------------------------------------------------------------------===// + +#if os(macOS) +import ContainerizationError +@preconcurrency import Foundation +import Logging +import Testing + +@testable import ContainerXPC + +@Suite(.timeLimit(.minutes(1)), .serialized) +struct XPCClientTests { + @Test + func replyBoxDeliversCancellationThatPrecedesContinuationStorage() async throws { + let box = XPCReplyBox() + box.resume { throw CancellationError() } + + do { + _ = try await withCheckedThrowingContinuation { continuation in + box.store(continuation) + } + Issue.record("expected the pending cancellation to be delivered") + } catch is CancellationError { + // Expected. + } + } + + @Test + func responseTimeoutReturnsWithinBound() async throws { + let server = AnonymousXPCServer() + defer { server.close() } + + let client = server.makeClient() + let clock = ContinuousClock() + let start = clock.now + + do { + _ = try await client.send( + XPCMessage(route: "hang"), + responseTimeout: .milliseconds(100) + ) + Issue.record("expected send to time out") + } catch let error as ContainerizationError { + #expect(error.message.contains("XPC timeout for request to test.container.xpc/hang")) + #expect(start.duration(to: clock.now) < .seconds(2)) + } + } + + @Test + func callerCancellationReturnsWithinBound() async throws { + let server = AnonymousXPCServer() + defer { server.close() } + + let client = server.makeClient() + let request = Task { + try await client.send( + XPCMessage(route: "hang"), + responseTimeout: .seconds(30) + ) + } + + try await Task.sleep(for: .milliseconds(50)) + let clock = ContinuousClock() + let start = clock.now + request.cancel() + + do { + _ = try await request.value + Issue.record("expected send to be cancelled") + } catch is CancellationError { + #expect(start.duration(to: clock.now) < .seconds(2)) + } + } + + @Test + func clientCanBeReusedAfterResponseTimeout() async throws { + let server = AnonymousXPCServer() + defer { server.close() } + + let client = server.makeClient() + + do { + _ = try await client.send( + XPCMessage(route: "hang"), + responseTimeout: .milliseconds(100) + ) + Issue.record("expected send to time out") + } catch let error as ContainerizationError { + #expect(error.message.contains("XPC timeout")) + } + + let response = try await client.send( + XPCMessage(route: "echo"), + responseTimeout: .seconds(1) + ) + #expect(response.string(key: "result") == "ok") + } + + @Test + func lateReplyAfterTimeoutIsIgnored() async throws { + let server = AnonymousXPCServer() + defer { server.close() } + + let client = server.makeClient() + + do { + _ = try await client.send( + XPCMessage(route: "hang"), + responseTimeout: .milliseconds(100) + ) + Issue.record("expected send to time out") + } catch let error as ContainerizationError { + #expect(error.message.contains("XPC timeout")) + } + + #expect(server.replyToPendingRequests()) + try await Task.sleep(for: .milliseconds(50)) + + let response = try await client.send( + XPCMessage(route: "echo"), + responseTimeout: .seconds(1) + ) + #expect(response.string(key: "result") == "ok") + } + + @Test + func unknownRouteReturnsInvalidArgument() async throws { + let listener = xpc_connection_create(nil, nil) + let server = XPCServer( + connection: listener, + routes: [:], + log: Logger(label: "test.container.xpc.unknown-route") + ) + let serverTask = Task { + try await server.listen() + } + defer { + serverTask.cancel() + xpc_connection_cancel(listener) + } + + try await Task.sleep(for: .milliseconds(50)) + let endpoint = xpc_endpoint_create(listener) + let client = XPCClient( + connection: xpc_connection_create_from_endpoint(endpoint), + label: "test.container.xpc" + ) + + do { + _ = try await client.send(XPCMessage(route: "missing")) + Issue.record("expected the server to reject an unknown route") + } catch let error as ContainerizationError { + #expect(error.code == .invalidArgument) + #expect(error.message == "unknown route: missing") + } + } +} + +private final class AnonymousXPCServer: @unchecked Sendable { + private let listener: xpc_connection_t + private let lock = NSLock() + private var connections = [xpc_connection_t]() + private var pendingRequests = [xpc_object_t]() + + init() { + listener = xpc_connection_create(nil, nil) + xpc_connection_set_event_handler(listener) { [weak self] object in + switch xpc_get_type(object) { + case XPC_TYPE_CONNECTION: + self?.accept(connection: object) + case XPC_TYPE_ERROR: + break + default: + fatalError("unhandled xpc object type: \(xpc_get_type(object))") + } + } + xpc_connection_activate(listener) + } + + func makeClient() -> XPCClient { + let endpoint = xpc_endpoint_create(listener) + let connection = xpc_connection_create_from_endpoint(endpoint) + return XPCClient(connection: connection, label: "test.container.xpc") + } + + func close() { + xpc_connection_cancel(listener) + lock.withLock { + for connection in connections { + xpc_connection_cancel(connection) + } + connections.removeAll() + pendingRequests.removeAll() + } + } + + func replyToPendingRequests() -> Bool { + let requests = lock.withLock { + let requests = pendingRequests + pendingRequests.removeAll() + return requests + } + guard let connection = lock.withLock({ connections.last }) else { + return false + } + for request in requests { + guard let reply = xpc_dictionary_create_reply(request) else { + continue + } + xpc_dictionary_set_string(reply, "result", "late") + xpc_connection_send_message(connection, reply) + } + return !requests.isEmpty + } + + private func accept(connection: xpc_connection_t) { + lock.withLock { + connections.append(connection) + } + nonisolated(unsafe) let connection = connection + xpc_connection_set_event_handler(connection) { [weak self] object in + guard let self else { + return + } + switch xpc_get_type(object) { + case XPC_TYPE_DICTIONARY: + let message = XPCMessage(object: object) + switch message.string(key: XPCMessage.routeKey) { + case "echo": + guard let reply = xpc_dictionary_create_reply(object) else { + return + } + xpc_dictionary_set_string(reply, "result", "ok") + xpc_connection_send_message(connection, reply) + default: + self.lock.withLock { + self.pendingRequests.append(object) + } + } + case XPC_TYPE_ERROR: + break + default: + fatalError("unhandled xpc object type: \(xpc_get_type(object))") + } + } + xpc_connection_activate(connection) + } +} +#endif From f4f908606d4a7693f9bfc958593f7671b35caa0f Mon Sep 17 00:00:00 2001 From: Steve Date: Sat, 11 Jul 2026 18:51:49 +0100 Subject: [PATCH 3/3] fix(plugins): keep root help responsive Use a one-second API health deadline for container --help, container help, and bare container while preserving the normal ten-second plugin dispatch deadline. Fall back to built-in help when the daemon is unavailable. Depends on the cancellable XPC request handling from apple/container#1862 and supersedes apple/container#1838 for apple/container#1459. --- Package.swift | 1 + Sources/ContainerCommands/Application.swift | 29 ++++++++- .../ContainerCommands/DefaultCommand.swift | 6 +- Sources/ContainerCommands/HelpCommand.swift | 2 +- .../ApplicationPluginDiscoveryTests.swift | 65 +++++++++++++++++++ .../IntegrationTests/System/TestCLIHelp.swift | 22 +++++++ 6 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 Tests/ContainerCommandsTests/ApplicationPluginDiscoveryTests.swift diff --git a/Package.swift b/Package.swift index 81086f9b2..ddf6c4014 100644 --- a/Package.swift +++ b/Package.swift @@ -153,6 +153,7 @@ let package = Package( .testTarget( name: "ContainerCommandsTests", dependencies: [ + "ContainerAPIClient", "ContainerCommands", "ContainerResource", ] diff --git a/Sources/ContainerCommands/Application.swift b/Sources/ContainerCommands/Application.swift index c77f51a59..2b7b45378 100644 --- a/Sources/ContainerCommands/Application.swift +++ b/Sources/ContainerCommands/Application.swift @@ -36,6 +36,8 @@ private nonisolated(unsafe) var bootstrapLogger = { return log }() +typealias APIServerHealthCheck = @Sendable (Duration) async throws -> SystemHealth + public struct Application: AsyncLoggableCommand { @OptionGroup public var logOptions: Flags.Logging @@ -129,7 +131,7 @@ public struct Application: AsyncLoggableCommand { // by ArgumentParser and lands here. let containsHelp = fullArgs.contains("-h") || fullArgs.contains("--help") if fullArgs.count <= 2 && containsHelp { - let pluginLoader = try? await createPluginLoader() + let pluginLoader = await pluginLoaderForHelp() await Self.printModifiedHelpText(pluginLoader: pluginLoader) return } @@ -144,6 +146,25 @@ public struct Application: AsyncLoggableCommand { } public static func createPluginLoader() async throws -> PluginLoader { + try await createPluginLoader( + healthTimeout: .seconds(10), + healthCheck: defaultAPIServerHealthCheck(timeout:) + ) + } + + static func pluginLoaderForHelp( + healthCheck: @escaping APIServerHealthCheck = defaultAPIServerHealthCheck(timeout:) + ) async -> PluginLoader? { + try? await createPluginLoader( + healthTimeout: .seconds(1), + healthCheck: healthCheck + ) + } + + private static func createPluginLoader( + healthTimeout: Duration, + healthCheck: @escaping APIServerHealthCheck + ) async throws -> PluginLoader { let installRootPath = CommandLine.executablePath .removingLastComponent() .removingLastComponent() @@ -175,7 +196,7 @@ public struct Application: AsyncLoggableCommand { AppBundlePluginFactory(logger: bootstrapLogger), ] - guard let systemHealth = try? await ClientHealthCheck.ping(timeout: .seconds(10)) else { + guard let systemHealth = try? await healthCheck(healthTimeout) else { throw ContainerizationError(.timeout, message: "unable to retrieve application data root from API server") } return try PluginLoader( @@ -188,6 +209,10 @@ public struct Application: AsyncLoggableCommand { ) } + private static func defaultAPIServerHealthCheck(timeout: Duration) async throws -> SystemHealth { + try await ClientHealthCheck.ping(timeout: timeout) + } + /// Load the system configuration using `appRoot` / `installRoot` reported by the /// daemon. `container system start` MUST have previously been run to start the daemon. public static func loadContainerSystemConfig() async throws -> ContainerSystemConfig { diff --git a/Sources/ContainerCommands/DefaultCommand.swift b/Sources/ContainerCommands/DefaultCommand.swift index 3df8f570e..956c4a98c 100644 --- a/Sources/ContainerCommands/DefaultCommand.swift +++ b/Sources/ContainerCommands/DefaultCommand.swift @@ -34,13 +34,15 @@ struct DefaultCommand: AsyncLoggableCommand { var remaining: [String] = [] func run() async throws { - // See if we have a possible plugin command. - let pluginLoader = try? await Application.createPluginLoader() guard let command = remaining.first else { + let pluginLoader = await Application.pluginLoaderForHelp() await Application.printModifiedHelpText(pluginLoader: pluginLoader) return } + // See if we have a possible plugin command. + let pluginLoader = try? await Application.createPluginLoader() + // Check for edge cases and unknown options to match the behavior in the absence of plugins. if command.isEmpty { throw ValidationError("unknown argument '\(command)'") diff --git a/Sources/ContainerCommands/HelpCommand.swift b/Sources/ContainerCommands/HelpCommand.swift index 43e9107c5..cf6458402 100644 --- a/Sources/ContainerCommands/HelpCommand.swift +++ b/Sources/ContainerCommands/HelpCommand.swift @@ -31,7 +31,7 @@ struct HelpCommand: AsyncLoggableCommand { func run() async throws { if subcommandPath.isEmpty { - let pluginLoader = try? await Application.createPluginLoader() + let pluginLoader = await Application.pluginLoaderForHelp() await Application.printModifiedHelpText(pluginLoader: pluginLoader) return } diff --git a/Tests/ContainerCommandsTests/ApplicationPluginDiscoveryTests.swift b/Tests/ContainerCommandsTests/ApplicationPluginDiscoveryTests.swift new file mode 100644 index 000000000..7ade86c21 --- /dev/null +++ b/Tests/ContainerCommandsTests/ApplicationPluginDiscoveryTests.swift @@ -0,0 +1,65 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// 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 ContainerAPIClient +import Foundation +import Testing + +@testable import ContainerCommands + +struct ApplicationPluginDiscoveryTests { + @Test + func helpPluginDiscoveryUsesShortHealthDeadline() async throws { + let appRoot = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + try FileManager.default.createDirectory(at: appRoot, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: appRoot) } + + let loader = await Application.pluginLoaderForHelp { timeout in + #expect(timeout == .seconds(1)) + return try Self.makeSystemHealth(appRoot: appRoot) + } + + #expect(loader != nil) + } + + @Test + func helpPluginDiscoveryReturnsNilWhenHealthCheckFails() async { + let loader = await Application.pluginLoaderForHelp { timeout in + #expect(timeout == .seconds(1)) + throw TestFailure.unavailable + } + + #expect(loader == nil) + } + + private static func makeSystemHealth(appRoot: URL) throws -> SystemHealth { + let json = """ + { + "appRoot": "\(appRoot.absoluteString)", + "installRoot": "file:///tmp/container-test-install-root", + "apiServerVersion": "test-version", + "apiServerCommit": "test-commit", + "apiServerBuild": "debug", + "apiServerAppName": "container-apiserver" + } + """ + return try JSONDecoder().decode(SystemHealth.self, from: Data(json.utf8)) + } +} + +private enum TestFailure: Error { + case unavailable +} diff --git a/Tests/IntegrationTests/System/TestCLIHelp.swift b/Tests/IntegrationTests/System/TestCLIHelp.swift index c91f6bc61..d4255ee32 100644 --- a/Tests/IntegrationTests/System/TestCLIHelp.swift +++ b/Tests/IntegrationTests/System/TestCLIHelp.swift @@ -18,6 +18,17 @@ import Testing @Suite struct TestCLIHelp { + @Test func testRootHelp() async throws { + try await ContainerFixture.with { f in + let result = try f.run(["--help"]) + #expect(result.status == 0, "root help should succeed, stderr: \(result.error)") + #expect( + result.output.contains("OVERVIEW: A container platform for macOS"), + "output should contain overview section" + ) + } + } + @Test func testHelp() async throws { try await ContainerFixture.with { f in let result = try f.run(["help"]) @@ -29,6 +40,17 @@ struct TestCLIHelp { } } + @Test func testDefaultHelp() async throws { + try await ContainerFixture.with { f in + let result = try f.run([]) + #expect(result.status == 0, "default help should succeed, stderr: \(result.error)") + #expect( + result.output.contains("OVERVIEW: A container platform for macOS"), + "output should contain overview section" + ) + } + } + @Test func testDebugHelp() async throws { try await ContainerFixture.with { f in let result = try f.run(["--debug", "help"])