From 872583eca8b4e7ae6ef85917604d4a5257b9d125 Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 22 Jul 2026 21:43:42 -0500 Subject: fix: adopt Swift 6 language mode; resolve all concurrency issues (#27) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three product targets (app, widget, share extension) now build under SWIFT_VERSION = 6.0 with zero errors and zero warnings. The UITests target stays on 5.0: XCTestCase's nonisolated setUp/init overrides conflict with the target's MainActor default isolation under 6, and test tooling is not shipping code. The original seven diagnostics, plus the layers Swift 6 mode surfaced once those cleared: - SMTPChannel is an actor. It was implicitly MainActor while running its receive loop on a background queue, so parsedLines/lineWaiters/ receiveBuffer were declared main-actor-protected and mutated off it — concurrent mutation while resuming a CheckedContinuation can double-resume, which traps. The actor serialises all state; Network callbacks hop in via Task. The start() continuation also gains an OSAllocatedUnfairLock resume-once guard: the state handler can fire .ready and later .failed, and resuming twice was a pre-existing trap of the same family. - CachedLookupResult is nonisolated (a value pair built inside actor LookupRuntime cannot have a MainActor-bound memberwise init) with conditional Sendable — opting out of MainActor isolation also opted out of the implicit Sendable that globally-isolated types get. - PortScanService.printableBanner is nonisolated: a pure transformation called from the connection's queue. - SweepActivityController stores the activity's Sendable id instead of the non-Sendable Activity, re-resolving via Activity.activities inside each fire-and-forget task, so nothing non-Sendable crosses isolation. - App Intents' static title/description/openAppWhenRun become lets (get-only protocol requirements; static var is shared mutable global state), and the summary helpers are @MainActor to match the model properties they read and the perform() implementations that call them. - ExternalDataService's ISO8601DateFormatter is nonisolated(unsafe), citing Apple's documented thread-safety, rather than risking a parser behaviour change by switching APIs with no test coverage. - TaskMetricsDelegate.metrics is nonisolated(unsafe): written on the session's delegate queue, read only after the request completes, and URLSession guarantees didFinishCollecting precedes task completion. - The share extension extracts the host via async/withCheckedContinuation instead of sending a non-Sendable completion into loadItem's @Sendable handler; Task inherits the view controller's MainActor so the manual DispatchQueue.main hop goes too. Validated: clean Swift 6 build of all product targets, and the full enforced 11-test audit suite green on the floor runtime — Swift 6's runtime isolation checks ran the app through every screen without a trap. --- DomainDig.xcodeproj/project.pbxproj | 12 ++-- DomainDig/DomainDigIntents.swift | 20 +++--- DomainDig/ExternalDataService.swift | 5 +- DomainDig/HTTPHeadersService.swift | 5 +- DomainDig/IntegrationService.swift | 74 +++++++++++++++-------- DomainDig/LookupRuntime.swift | 10 ++- DomainDig/PortScanService.swift | 5 +- DomainDig/SweepActivityController.swift | 39 ++++++++---- DomainDigShareExtension/ShareViewController.swift | 26 ++++---- 9 files changed, 129 insertions(+), 67 deletions(-) diff --git a/DomainDig.xcodeproj/project.pbxproj b/DomainDig.xcodeproj/project.pbxproj index 6b4c25c..c132d2a 100644 --- a/DomainDig.xcodeproj/project.pbxproj +++ b/DomainDig.xcodeproj/project.pbxproj @@ -591,7 +591,7 @@ SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = "1,2"; }; name = Debug; @@ -628,7 +628,7 @@ SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = "1,2"; }; name = Release; @@ -657,7 +657,7 @@ SWIFT_APPROACHABLE_CONCURRENCY = YES; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = "1,2"; }; name = Debug; @@ -686,7 +686,7 @@ SWIFT_APPROACHABLE_CONCURRENCY = YES; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = "1,2"; }; name = Release; @@ -715,7 +715,7 @@ SWIFT_APPROACHABLE_CONCURRENCY = YES; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = "1,2"; }; name = Debug; @@ -744,7 +744,7 @@ SWIFT_APPROACHABLE_CONCURRENCY = YES; SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.0; TARGETED_DEVICE_FAMILY = "1,2"; }; name = Release; diff --git a/DomainDig/DomainDigIntents.swift b/DomainDig/DomainDigIntents.swift index 8b46aa7..70b9e67 100644 --- a/DomainDig/DomainDigIntents.swift +++ b/DomainDig/DomainDigIntents.swift @@ -6,13 +6,13 @@ import Foundation /// `DomainReportBuilder`) and returns a concise summary. Usable from /// Shortcuts, Spotlight, the Action button, and Siri. struct InspectDomainIntent: AppIntent { - static var title: LocalizedStringResource = "Inspect Domain" - static var description = IntentDescription( + static let title: LocalizedStringResource = "Inspect Domain" + static let description = IntentDescription( "Run a DomainDig inspection and return a summary of availability, risk, TLS, email security, and certificate health." ) // Read-only inspection; no need to foreground the app. - static var openAppWhenRun = false + static let openAppWhenRun = false @Parameter( title: "Domain", @@ -44,6 +44,7 @@ struct InspectDomainIntent: AppIntent { } /// Multi-line summary suitable for a returned Shortcuts text value. + @MainActor static func summaryText(for report: DomainReport) -> String { let dnssec: String switch report.dns.dnssecSigned { @@ -68,6 +69,7 @@ struct InspectDomainIntent: AppIntent { } /// Short spoken/dialog line for Siri and the Shortcuts result banner. + @MainActor static func spokenSummary(for report: DomainReport) -> String { "\(report.domain) is \(report.availability.rawValue). Risk \(report.riskAssessment.level.title.lowercased()), health \(report.health.title.lowercased())." } @@ -89,12 +91,12 @@ enum InspectDomainError: Error, CustomLocalizedStringResourceConvertible { /// existing view-model path (premium limits, monitoring, history linking, /// cloud-sync recording, and the paywall when over the free limit). struct AddToWatchlistIntent: AppIntent { - static var title: LocalizedStringResource = "Add Domain to Watchlist" - static var description = IntentDescription( + static let title: LocalizedStringResource = "Add Domain to Watchlist" + static let description = IntentDescription( "Open DomainDig and add a domain to your watchlist." ) - static var openAppWhenRun = true + static let openAppWhenRun = true @Parameter( title: "Domain", @@ -128,12 +130,12 @@ struct AddToWatchlistIntent: AppIntent { /// through the existing view-model batch path (`refreshAllTrackedDomains`), which /// enforces the batch feature gate and surfaces the paywall when needed. struct RunSweepIntent: AppIntent { - static var title: LocalizedStringResource = "Run Watchlist Sweep" - static var description = IntentDescription( + static let title: LocalizedStringResource = "Run Watchlist Sweep" + static let description = IntentDescription( "Open DomainDig and re-inspect every domain on your watchlist." ) - static var openAppWhenRun = true + static let openAppWhenRun = true @MainActor func perform() async throws -> some IntentResult { diff --git a/DomainDig/ExternalDataService.swift b/DomainDig/ExternalDataService.swift index 43cc489..8f5b07e 100644 --- a/DomainDig/ExternalDataService.swift +++ b/DomainDig/ExternalDataService.swift @@ -738,7 +738,10 @@ actor ExternalDataService { } } - private static let iso8601DateFormatter: ISO8601DateFormatter = { + // ISO8601DateFormatter is documented thread-safe ("ISO8601DateFormatter is + // thread-safe" — Apple docs), so sharing one instance across contexts is + // sound; the annotation records that the compiler cannot see it. (#27) + private nonisolated(unsafe) static let iso8601DateFormatter: ISO8601DateFormatter = { let formatter = ISO8601DateFormatter() formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] return formatter diff --git a/DomainDig/HTTPHeadersService.swift b/DomainDig/HTTPHeadersService.swift index 4bf175a..bbc2842 100644 --- a/DomainDig/HTTPHeadersService.swift +++ b/DomainDig/HTTPHeadersService.swift @@ -103,7 +103,10 @@ struct HTTPHeadersService { } private final class TaskMetricsDelegate: NSObject, URLSessionTaskDelegate { - private(set) var metrics: URLSessionTaskMetrics? + // Written from the session's delegate queue, read only after the request + // has completed — URLSession guarantees didFinishCollecting is delivered + // before the task finishes, so the accesses are sequenced. (#27) + nonisolated(unsafe) private(set) var metrics: URLSessionTaskMetrics? func urlSession( _ _: URLSession, diff --git a/DomainDig/IntegrationService.swift b/DomainDig/IntegrationService.swift index de24c23..090f564 100644 --- a/DomainDig/IntegrationService.swift +++ b/DomainDig/IntegrationService.swift @@ -1,5 +1,6 @@ import Foundation import Network +import os import Observation import Security @@ -749,11 +750,17 @@ private enum SMTPClient { try await channel.sendRaw(body + "\r\n.\r\n") _ = try await channel.readResponse(expecting: [250]) _ = try await channel.sendCommand("QUIT", expecting: [221]) - channel.cancel() + await channel.cancel() } } -private final class SMTPChannel { +/// An actor, not a MainActor class. The previous shape inherited the project's +/// MainActor default while running its receive loop on a background dispatch +/// queue, so `parsedLines`/`lineWaiters`/`receiveBuffer` were declared +/// main-actor-protected and mutated off it — concurrent mutation while resuming +/// a `CheckedContinuation` can double-resume, which traps. The actor serialises +/// all of it and forces the Network callbacks to hop in explicitly. (Issue #27.) +private actor SMTPChannel { private let connection: NWConnection private var parsedLines: [String] = [] private var lineWaiters: [CheckedContinuation] = [] @@ -765,25 +772,35 @@ private final class SMTPChannel { func start() async throws { try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in - connection.stateUpdateHandler = { [weak self] state in + // The state handler can fire `.ready` and later `.failed` (or + // `.failed` twice); resuming a continuation twice traps. The lock + // also keeps the closure Sendable-clean without touching actor state + // from the connection's queue. + let hasResumed = OSAllocatedUnfairLock(initialState: false) + connection.stateUpdateHandler = { state in + let isFirst: () -> Bool = { + hasResumed.withLock { resumed in + if resumed { return false } + resumed = true + return true + } + } switch state { case .ready: - self?.scheduleReceiveLoop() - continuation.resume() + if isFirst() { continuation.resume() } case .failed(let error): - continuation.resume(throwing: error) + if isFirst() { continuation.resume(throwing: error) } + case .cancelled: + if isFirst() { continuation.resume(throwing: IntegrationError.streamClosed) } default: break } } connection.start(queue: .global(qos: .utility)) } - } - - private func scheduleReceiveLoop() { - DispatchQueue.global(qos: .utility).async { [weak self] in - self?.startReceiveLoop() - } + // Started from the actor once the connection is ready, replacing the old + // dispatch-queue hop. TCP buffers anything that arrives in the gap. + startReceiveLoop() } func cancel() { @@ -845,26 +862,33 @@ private final class SMTPChannel { } private func startReceiveLoop() { + // The completion runs on the connection's queue; hop back onto the + // actor before touching any state. connection.receive(minimumIncompleteLength: 1, maximumLength: 4096) { [weak self] data, _, isComplete, error in guard let self else { return } - - if let error { - self.failWaiters(with: error) - return + Task { + await self.handleReceive(data: data, isComplete: isComplete, error: error) } + } + } - if let data, !data.isEmpty { - self.receiveBuffer.append(data) - self.flushBuffer() - } + private func handleReceive(data: Data?, isComplete: Bool, error: Error?) { + if let error { + failWaiters(with: error) + return + } - if isComplete { - self.failWaiters(with: IntegrationError.streamClosed) - return - } + if let data, !data.isEmpty { + receiveBuffer.append(data) + flushBuffer() + } - self.startReceiveLoop() + if isComplete { + failWaiters(with: IntegrationError.streamClosed) + return } + + startReceiveLoop() } private func flushBuffer() { diff --git a/DomainDig/LookupRuntime.swift b/DomainDig/LookupRuntime.swift index 5db3134..7d9a8bd 100644 --- a/DomainDig/LookupRuntime.swift +++ b/DomainDig/LookupRuntime.swift @@ -1,10 +1,18 @@ import Foundation -struct CachedLookupResult { +/// Opted out of the project's MainActor default isolation: this is a plain +/// value pair constructed inside `actor LookupRuntime`, and a MainActor-bound +/// memberwise init cannot be called from there under Swift 6. +nonisolated struct CachedLookupResult { let value: Value let source: LookupResultSource } +/// Opting out of MainActor isolation also opted out of the implicit +/// Sendable that globally-isolated types get, which is what lets this cross +/// from `actor LookupRuntime` back to its callers. +extension CachedLookupResult: Sendable where Value: Sendable {} + actor LookupRuntime { static let shared = LookupRuntime() diff --git a/DomainDig/PortScanService.swift b/DomainDig/PortScanService.swift index 808b38a..31bf298 100644 --- a/DomainDig/PortScanService.swift +++ b/DomainDig/PortScanService.swift @@ -106,7 +106,10 @@ struct PortScanService { } } - private static func printableBanner(from data: Data?, error: Error?) -> String? { + /// Pure data transformation, called from the connection's background queue — + /// `nonisolated` opts it out of the project's MainActor default, which would + /// otherwise make this call a data-race diagnostic under Swift 6. + private nonisolated static func printableBanner(from data: Data?, error: Error?) -> String? { guard error == nil, let data, !data.isEmpty, diff --git a/DomainDig/SweepActivityController.swift b/DomainDig/SweepActivityController.swift index a00bb2f..e6d179e 100644 --- a/DomainDig/SweepActivityController.swift +++ b/DomainDig/SweepActivityController.swift @@ -2,11 +2,18 @@ import ActivityKit import Foundation /// Starts, updates, and ends the sweep Live Activity around a batch run. +/// +/// Holds the activity's `id` (a Sendable `String`) rather than the +/// `Activity` object itself. `Activity` is not Sendable, and sending the +/// stored reference into the fire-and-forget update task while `self` still +/// held it was a Swift 6 region-isolation violation (issue #27). Each task +/// re-resolves the activity via `Activity.activities`, ActivityKit's +/// sanctioned lookup, so nothing non-Sendable crosses an isolation boundary. @MainActor final class SweepActivityController { static let shared = SweepActivityController() - private var activity: Activity? + private var activityID: String? private init() { /* Singleton; use the shared instance. */ } @@ -23,14 +30,15 @@ final class SweepActivityController { changed: 0, warnings: 0 ) - activity = try? Activity.request( + let activity = try? Activity.request( attributes: SweepActivityAttributes(title: title, startedAt: Date()), content: ActivityContent(state: state, staleDate: nil) ) + activityID = activity?.id } func update(completed: Int, total: Int, currentDomain: String?) { - guard let activity else { return } + guard let activityID else { return } let state = SweepActivityAttributes.ContentState( completed: completed, total: total, @@ -39,25 +47,32 @@ final class SweepActivityController { warnings: 0 ) Task { + guard let activity = Self.activity(withID: activityID) else { return } await activity.update(ActivityContent(state: state, staleDate: nil)) } } func end(changed: Int, warnings: Int, immediately: Bool = false) { - guard let activity else { return } - self.activity = nil - let state = SweepActivityAttributes.ContentState( - completed: activity.content.state.total, - total: activity.content.state.total, - currentDomain: nil, - changed: changed, - warnings: warnings - ) + guard let activityID else { return } + self.activityID = nil Task { + guard let activity = Self.activity(withID: activityID) else { return } + let total = activity.content.state.total + let state = SweepActivityAttributes.ContentState( + completed: total, + total: total, + currentDomain: nil, + changed: changed, + warnings: warnings + ) await activity.end( ActivityContent(state: state, staleDate: nil), dismissalPolicy: immediately ? .immediate : .after(Date().addingTimeInterval(60)) ) } } + + private nonisolated static func activity(withID id: String) -> Activity? { + Activity.activities.first { $0.id == id } + } } diff --git a/DomainDigShareExtension/ShareViewController.swift b/DomainDigShareExtension/ShareViewController.swift index 411f9e7..1243892 100644 --- a/DomainDigShareExtension/ShareViewController.swift +++ b/DomainDigShareExtension/ShareViewController.swift @@ -24,14 +24,17 @@ final class ShareViewController: UIViewController { label.trailingAnchor.constraint(lessThanOrEqualTo: view.trailingAnchor, constant: -24) ]) - extractSharedDomain { [weak self] domain in - DispatchQueue.main.async { - self?.finish(domain: domain) - } + // Task inherits this view controller's MainActor context, so finish() + // lands back on main without a manual dispatch. The continuation form + // also avoids sending a non-Sendable completion into loadItem's + // @Sendable handler. (#27) + Task { [weak self] in + let domain = await self?.extractSharedDomain() + self?.finish(domain: domain ?? nil) } } - private func extractSharedDomain(completion: @escaping (String?) -> Void) { + private func extractSharedDomain() async -> String? { let providers = (extensionContext?.inputItems ?? []) .compactMap { $0 as? NSExtensionItem } .flatMap { $0.attachments ?? [] } @@ -39,14 +42,15 @@ final class ShareViewController: UIViewController { guard let provider = providers.first(where: { $0.hasItemConformingToTypeIdentifier(UTType.url.identifier) }) else { - completion(nil) - return + return nil } - provider.loadItem(forTypeIdentifier: UTType.url.identifier) { item, _ in - let url = item as? URL ?? (item as? Data).flatMap { URL(dataRepresentation: $0, relativeTo: nil) } - let host = url?.host?.trimmingCharacters(in: .whitespacesAndNewlines) - completion(host?.isEmpty == false ? host : nil) + return await withCheckedContinuation { continuation in + provider.loadItem(forTypeIdentifier: UTType.url.identifier) { item, _ in + let url = item as? URL ?? (item as? Data).flatMap { URL(dataRepresentation: $0, relativeTo: nil) } + let host = url?.host?.trimmingCharacters(in: .whitespacesAndNewlines) + continuation.resume(returning: host?.isEmpty == false ? host : nil) + } } } -- cgit v1.2.3