From 94a170089dbfbd4f9af0fa2fbc8f2e51aa5605b9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 01:01:24 -0700 Subject: [PATCH] fix(ios): prevent Apple Watch message callbacks from crashing the app (#129921) --- .../Services/WatchConnectivityTransport.swift | 21 ------- .../Services/WatchSessionActivationGate.swift | 34 ++++++++++++ .../WatchSessionActivationGateTests.swift | 55 +++++++++++++++++-- .../Sources/WatchConnectivityReceiver.swift | 21 +------ 4 files changed, 85 insertions(+), 46 deletions(-) diff --git a/apps/ios/Sources/Services/WatchConnectivityTransport.swift b/apps/ios/Sources/Services/WatchConnectivityTransport.swift index 0a4c40c55210..f409c9e215d4 100644 --- a/apps/ios/Sources/Services/WatchConnectivityTransport.swift +++ b/apps/ios/Sources/Services/WatchConnectivityTransport.swift @@ -11,27 +11,6 @@ private struct WatchConnectivityTransportCallbacks { var appCommandHandler: (@Sendable (WatchAppCommandEvent) -> Void)? } -private func sendReachableWatchMessage(_ payload: [String: Any], with session: WCSession) async throws { - // WatchConnectivity replies arrive on its own queue. Keep this continuation explicitly - // nonisolated so Swift 6 does not inherit a caller actor (for example MainActor) into the - // Objective-C callback boundary and trap on the reply callback executor check. - try await withCheckedThrowingContinuation(isolation: nil) { (continuation: CheckedContinuation) in - session.sendMessage( - payload, - replyHandler: { reply in - do { - try requireAcceptedWatchMessageReply(reply) - continuation.resume(returning: ()) - } catch { - continuation.resume(throwing: error) - } - }, - errorHandler: { error in - continuation.resume(throwing: error) - }) - } -} - final class WatchConnectivityTransport: NSObject, @unchecked Sendable { private nonisolated static let logger = Logger(subsystem: "ai.openclawfoundation.app", category: "watch.messaging") diff --git a/apps/ios/Sources/Services/WatchSessionActivationGate.swift b/apps/ios/Sources/Services/WatchSessionActivationGate.swift index 3593bc518e08..169b098aa77e 100644 --- a/apps/ios/Sources/Services/WatchSessionActivationGate.swift +++ b/apps/ios/Sources/Services/WatchSessionActivationGate.swift @@ -1,4 +1,5 @@ import Foundation +@preconcurrency import WatchConnectivity enum WatchMessageAcknowledgmentError: LocalizedError { case rejected(String) @@ -23,6 +24,39 @@ func requireAcceptedWatchMessageReply(_ reply: [String: Any]) throws { } } +final class WatchMessageSendCompletion: @unchecked Sendable { + private let lock = NSLock() + private var continuation: CheckedContinuation? + + init(_ continuation: CheckedContinuation) { + self.continuation = continuation + } + + func complete(_ result: Result) { + let continuation = self.lock.withLock { () -> CheckedContinuation? in + defer { self.continuation = nil } + return self.continuation + } + continuation?.resume(with: result) + } +} + +func sendReachableWatchMessage(_ payload: [String: Any], with session: WCSession) async throws { + // WatchConnectivity callbacks use their own executor and can race despite their + // documented exactly-once contract; only the first callback owns this continuation. + try await withCheckedThrowingContinuation(isolation: nil) { continuation in + let completion = WatchMessageSendCompletion(continuation) + session.sendMessage( + payload, + replyHandler: { reply in + completion.complete(Result { try requireAcceptedWatchMessageReply(reply) }) + }, + errorHandler: { error in + completion.complete(.failure(error)) + }) + } +} + enum WatchSessionActivationError: LocalizedError { case failed(String) case timedOut diff --git a/apps/ios/Tests/WatchSessionActivationGateTests.swift b/apps/ios/Tests/WatchSessionActivationGateTests.swift index cbaeefa1036a..0c19a5736cfa 100644 --- a/apps/ios/Tests/WatchSessionActivationGateTests.swift +++ b/apps/ios/Tests/WatchSessionActivationGateTests.swift @@ -17,6 +17,55 @@ struct WatchSessionActivationGateTests { } } + @Test func `accepted watch reply ignores later transport callbacks`() async throws { + try await withCheckedThrowingContinuation { continuation in + let completion = WatchMessageSendCompletion(continuation) + completion.complete(.success(())) + completion.complete(.failure(URLError(.timedOut))) + completion.complete(.success(())) + } + } + + @Test func `watch transport error ignores later replies and errors`() async { + await #expect(throws: URLError.self) { + try await withCheckedThrowingContinuation { continuation in + let completion = WatchMessageSendCompletion(continuation) + completion.complete(.failure(URLError(.notConnectedToInternet))) + completion.complete(.success(())) + completion.complete(.failure(WatchMessageAcknowledgmentError.rejected("late"))) + } + } + } + + @Test func `rejected watch acknowledgment ignores a later transport error`() async { + await #expect(throws: WatchMessageAcknowledgmentError.self) { + try await withCheckedThrowingContinuation { continuation in + let completion = WatchMessageSendCompletion(continuation) + completion.complete(Result { + try requireAcceptedWatchMessageReply(["ok": false, "error": "unsupported_payload"]) + }) + completion.complete(.failure(URLError(.timedOut))) + } + } + } + + @Test func `racing watch callbacks complete their continuation exactly once`() async { + do { + try await withCheckedThrowingContinuation { continuation in + let completion = WatchMessageSendCompletion(continuation) + DispatchQueue.concurrentPerform(iterations: 100) { index in + completion.complete(index.isMultiple(of: 2) + ? .success(()) + : .failure(URLError(.timedOut))) + } + } + } catch is URLError { + // Either terminal callback may win; racing callbacks must never resume twice. + } catch { + Issue.record("Unexpected watch message failure: \(error)") + } + } + @Test func `startup event buffering is ordered and bounded`() { var buffer = WatchMessagingStartupBuffer(maxCount: 3) @@ -87,10 +136,6 @@ struct WatchSessionActivationGateTests { contentsOf: iosRoot.appendingPathComponent( "WatchApp/Sources/WatchConnectivityReceiver.swift"), encoding: .utf8) - let transportSource = try String( - contentsOf: iosRoot.appendingPathComponent( - "Sources/Services/WatchConnectivityTransport.swift"), - encoding: .utf8) let serviceSource = try String( contentsOf: iosRoot.appendingPathComponent( "Sources/Services/WatchMessagingService.swift"), @@ -104,8 +149,6 @@ struct WatchSessionActivationGateTests { #expect(receiverSource.contains( "acknowledgment: WatchMessageAcknowledgment? = nil) -> Bool")) #expect(receiverSource.contains("guard activationState == .activated else { return }")) - #expect(receiverSource.contains("try requireAcceptedWatchMessageReply(reply)")) - #expect(transportSource.contains("try requireAcceptedWatchMessageReply(reply)")) let callbackRegistration = try #require( serviceSource.range(of: "self.transport.setAppCommandHandler")) let activation = try #require(serviceSource.range(of: "self.transport.activate()")) diff --git a/apps/ios/WatchApp/Sources/WatchConnectivityReceiver.swift b/apps/ios/WatchApp/Sources/WatchConnectivityReceiver.swift index a47e32f1a416..fdc978817980 100644 --- a/apps/ios/WatchApp/Sources/WatchConnectivityReceiver.swift +++ b/apps/ios/WatchApp/Sources/WatchConnectivityReceiver.swift @@ -92,7 +92,6 @@ struct WatchExecApprovalSnapshotRequestToken: Hashable, Sendable { } final class WatchConnectivityReceiver: NSObject, @unchecked Sendable { - private typealias MessageSendContinuation = CheckedContinuation private static let maxAcceptedExecApprovalSnapshotRequests = 32 private let store: WatchInboxStore @@ -165,7 +164,7 @@ final class WatchConnectivityReceiver: NSObject, @unchecked Sendable { let payload = Self.encodeSnapshotRequestPayload(request) if session.isReachable { do { - try await Self.sendMessage(payload, through: session) + try await sendReachableWatchMessage(payload, with: session) return token } catch { // Fall through to queued delivery. @@ -261,7 +260,7 @@ final class WatchConnectivityReceiver: NSObject, @unchecked Sendable { var requiresCanonicalReadback = false if session.isReachable { do { - try await Self.sendMessage(payload, through: session) + try await sendReachableWatchMessage(payload, with: session) return WatchReplySendResult( delivery: .delivered, transport: "sendMessage", @@ -283,22 +282,6 @@ final class WatchConnectivityReceiver: NSObject, @unchecked Sendable { requiresCanonicalReadback: requiresCanonicalReadback) } - private static func sendMessage(_ payload: [String: Any], through session: WCSession) async throws { - try await withCheckedThrowingContinuation(isolation: nil) { (continuation: MessageSendContinuation) in - session.sendMessage( - payload, - replyHandler: { reply in - do { - try requireAcceptedWatchMessageReply(reply) - continuation.resume(returning: ()) - } catch { - continuation.resume(throwing: error) - } - }, - errorHandler: { error in continuation.resume(throwing: error) }) - } - } - private static func unavailableResult(_ error: any Error) -> WatchReplySendResult { // Activation failed before a payload could be handed to WatchConnectivity. // The closed notSent state lets callers safely offer an immediate retry.