From 6813d8584fc5d0a3ee5914700f45b1cb3bef36ff Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 26 Jun 2026 22:37:32 -0400 Subject: [PATCH] Make the codebase clean under SWIFT_STRICT_CONCURRENCY=complete Clears the data-race-checking warnings that blocked enabling complete concurrency checking (a prerequisite for the Swift 6 language mode) and turns the setting on in project.yml so any new issue surfaces immediately. - Theme and its token types are now Sendable; Typography's font closures are marked @Sendable (the one non-Sendable member that broke the chain), which also makes the Theme statics and ThemeEnvironment.defaultValue safe. - Catalog.isoTimestamp is a computed property rather than a shared, non-Sendable ISO8601DateFormatter static. - The KVO status/rate observers re-capture [weak self] on the inner DispatchQueue.main.async closure and run their body in MainActor.assumeIsolated, so self is no longer captured from the outer closure's weak var in concurrently-executing code. - The didPlayToEndTime handler sends an ObjectIdentifier across the main hop instead of the non-Sendable Notification / AVPlayerItem. Verified: a full from-scratch build under SWIFT_STRICT_CONCURRENCY=complete is warning-free, and a SWIFT_STRICT_CONCURRENCY=minimal build still succeeds (no regression to the non-strict build). Co-Authored-By: Claude Opus 4.8 --- AppleNugs/Core/Catalog.swift | 9 ++-- AppleNugs/Player/PlayerService.swift | 64 ++++++++++++++--------- AppleNugs/Player/VideoPlayerService.swift | 64 ++++++++++++----------- AppleNugs/Theme/Theme.swift | 20 +++---- project.yml | 4 ++ 5 files changed, 92 insertions(+), 69 deletions(-) diff --git a/AppleNugs/Core/Catalog.swift b/AppleNugs/Core/Catalog.swift index 21059fa..0a94416 100644 --- a/AppleNugs/Core/Catalog.swift +++ b/AppleNugs/Core/Catalog.swift @@ -433,12 +433,15 @@ enum Catalog { } /// Full timestamp parsing for REST `/livestreams` event times. Tolerates - /// both fractional and whole-second ISO-8601 (with or without "Z"). - private static let isoTimestamp: ISO8601DateFormatter = { + /// both fractional and whole-second ISO-8601 (with or without "Z"). A + /// computed property rather than a shared `static let`: `ISO8601DateFormatter` + /// isn't `Sendable`, and this is called rarely enough (a handful of event + /// times per livestream load) that a fresh formatter per call is free. + private static var isoTimestamp: ISO8601DateFormatter { let f = ISO8601DateFormatter() f.formatOptions = [.withInternetDateTime, .withFractionalSeconds] return f - }() + } static func parseTimestamp(_ raw: String?) -> Date? { guard let raw, !raw.isEmpty else { return nil } diff --git a/AppleNugs/Player/PlayerService.swift b/AppleNugs/Player/PlayerService.swift index f0200bc..a9bb4c0 100644 --- a/AppleNugs/Player/PlayerService.swift +++ b/AppleNugs/Player/PlayerService.swift @@ -142,8 +142,12 @@ final class PlayerService { endObserver = NotificationCenter.default.addObserver( forName: AVPlayerItem.didPlayToEndTimeNotification, object: nil, queue: .main) { [weak self] note in + // Send only the ended item's identity across the actor hop — a + // `Notification` (and the `AVPlayerItem` it carries) isn't Sendable, + // and `itemDidEnd` only needs identity to match the current item. + let endedID = (note.object as? AVPlayerItem).map(ObjectIdentifier.init) MainActor.assumeIsolated { - self?.itemDidEnd(note.object as? AVPlayerItem) + self?.itemDidEnd(endedID) } } terminationObserver = NotificationCenter.default.addObserver( @@ -422,22 +426,28 @@ final class PlayerService { private func observeCurrent(_ item: AVPlayerItem) { statusObservation = item.observe(\.status, options: [.new]) { [weak self] item, _ in - DispatchQueue.main.async { - guard let self, item === self.currentItem else { return } - switch item.status { - case .failed: - // Signed URL rejected or format undecodable here — fall - // through to the next available format for this track, - // and drop the cached picks so a retry re-probes. - if let track = self.current { - Task { await self.client.invalidateStreams(for: track.trackId) } + // KVO fires on an arbitrary AVFoundation queue, so hop to main. The + // inner closure captures `self` weakly itself (rather than closing + // over the outer closure's capture, which `@Sendable` checking flags) + // and `assumeIsolated` supplies the main-actor isolation it lands on. + DispatchQueue.main.async { [weak self] in + MainActor.assumeIsolated { + guard let self, item === self.currentItem else { return } + switch item.status { + case .failed: + // Signed URL rejected or format undecodable here — fall + // through to the next available format for this track, + // and drop the cached picks so a retry re-probes. + if let track = self.current { + Task { await self.client.invalidateStreams(for: track.trackId) } + } + self.pickIndex += 1 + self.loadCurrentPick() + case .readyToPlay: + self.applyPendingSeek() + default: + break } - self.pickIndex += 1 - self.loadCurrentPick() - case .readyToPlay: - self.applyPendingSeek() - default: - break } } } @@ -454,8 +464,8 @@ final class PlayerService { } /// Fired whenever any of our items plays to the end. - private func itemDidEnd(_ item: AVPlayerItem?) { - guard let item, item === currentItem else { return } + private func itemDidEnd(_ itemID: ObjectIdentifier?) { + guard let itemID, let currentItem, itemID == ObjectIdentifier(currentItem) else { return } if let p = preload, queue.indices.contains(index + 1), queue[index + 1].id == p.trackUID { // AVQueuePlayer has already advanced into the preloaded item — @@ -535,14 +545,16 @@ final class PlayerService { } let item = makeItem(url: url) let observation = item.observe(\.status, options: [.new]) { [weak self] item, _ in - DispatchQueue.main.async { - guard let self, self.preload?.item === item else { return } - if item.status == .failed { - // The parked pick failed before playback — swap in the - // next format without disturbing the playing track. - Task { await self.client.invalidateStreams(for: track.trackId) } - self.discardPreload() - self.buildPreload(track: track, picks: picks, startingAt: pickIdx + 1) + DispatchQueue.main.async { [weak self] in + MainActor.assumeIsolated { + guard let self, self.preload?.item === item else { return } + if item.status == .failed { + // The parked pick failed before playback — swap in the + // next format without disturbing the playing track. + Task { await self.client.invalidateStreams(for: track.trackId) } + self.discardPreload() + self.buildPreload(track: track, picks: picks, startingAt: pickIdx + 1) + } } } } diff --git a/AppleNugs/Player/VideoPlayerService.swift b/AppleNugs/Player/VideoPlayerService.swift index 8718069..9b60cfc 100644 --- a/AppleNugs/Player/VideoPlayerService.swift +++ b/AppleNugs/Player/VideoPlayerService.swift @@ -109,12 +109,14 @@ final class VideoPlayerService { // native play button replaying a finished video) — otherwise video // audio would play underneath resumed audio with media keys misrouted. rateObservation = player.observe(\.timeControlStatus, options: [.new]) { [weak self] player, _ in - DispatchQueue.main.async { - guard let self, self.current != nil, !self.ownsNowPlaying, - player.timeControlStatus == .playing else { return } - self.claimArbiterIfNeeded() - self.isPlaying = true - self.pushNowPlayingInfo() + DispatchQueue.main.async { [weak self] in + MainActor.assumeIsolated { + guard let self, self.current != nil, !self.ownsNowPlaying, + player.timeControlStatus == .playing else { return } + self.claimArbiterIfNeeded() + self.isPlaying = true + self.pushNowPlayingInfo() + } } } } @@ -325,30 +327,32 @@ final class VideoPlayerService { private func observe(_ item: AVPlayerItem) { statusObservation = item.observe(\.status, options: [.new]) { [weak self] item, _ in - DispatchQueue.main.async { - guard let self, item === self.item else { return } - switch item.status { - case .failed: - self.loadError = item.error?.localizedDescription - ?? "This video failed to play." - // The video can't play. Detach the dead item and clear - // `current` BEFORE relinquishing: leaving it attached to the - // native player controls would let the timeControlStatus - // observer (guarded on `current != nil`) re-claim the arbiter - // and re-pause the audio we're about to hand back. Clearing - // currentTime also avoids a stale "current chapter" highlight; - // loadError still drives the error overlay, and `current == - // nil` turns the Play button into a retry. - self.currentTime = 0 - self.isPlaying = false - self.loadGeneration += 1 // cancel any in-flight variant load - self.tearDownItem() - self.current = nil - self.relinquishArbiter() - case .readyToPlay: - self.applyPendingStartSeek() - default: - break + DispatchQueue.main.async { [weak self] in + MainActor.assumeIsolated { + guard let self, item === self.item else { return } + switch item.status { + case .failed: + self.loadError = item.error?.localizedDescription + ?? "This video failed to play." + // The video can't play. Detach the dead item and clear + // `current` BEFORE relinquishing: leaving it attached to the + // native player controls would let the timeControlStatus + // observer (guarded on `current != nil`) re-claim the arbiter + // and re-pause the audio we're about to hand back. Clearing + // currentTime also avoids a stale "current chapter" highlight; + // loadError still drives the error overlay, and `current == + // nil` turns the Play button into a retry. + self.currentTime = 0 + self.isPlaying = false + self.loadGeneration += 1 // cancel any in-flight variant load + self.tearDownItem() + self.current = nil + self.relinquishArbiter() + case .readyToPlay: + self.applyPendingStartSeek() + default: + break + } } } } diff --git a/AppleNugs/Theme/Theme.swift b/AppleNugs/Theme/Theme.swift index 7953584..ee543ec 100644 --- a/AppleNugs/Theme/Theme.swift +++ b/AppleNugs/Theme/Theme.swift @@ -3,7 +3,7 @@ import SwiftUI // MARK: - Variation axes /// Which bespoke now-playing treatment a theme uses in the transport bar. -enum TransportSignature { +enum TransportSignature: Sendable { case standard // the original text block, token-styled case tapeLabel // Tape Room: art chip + amber under-rule progress case jCard // Shoebox: cassette J-card strip @@ -11,13 +11,13 @@ enum TransportSignature { } /// Whether the accent is a fixed color or derived from the current cover art. -enum AccentMode { +enum AccentMode: Sendable { case staticAccent case artDriven(fallback: Color) } /// How (if at all) the extracted album-art color washes into the chrome. -enum WashStyle { +enum WashStyle: Sendable { case none case linear // Soundboard: gradient behind transport + now-playing case warmLow // Shoebox: faint warm wash @@ -25,7 +25,7 @@ enum WashStyle { } /// Opt-in bespoke behaviors, kept off the host views so nothing forks. -struct Capabilities: OptionSet { +struct Capabilities: OptionSet, Sendable { let rawValue: Int static let artWash = Capabilities(rawValue: 1 << 0) static let equalizerRows = Capabilities(rawValue: 1 << 1) @@ -37,7 +37,7 @@ struct Capabilities: OptionSet { // MARK: - Token groups /// Semantic color roles. Views ask for `textSecondary`, never "taupe". -struct Palette { +struct Palette: Sendable { let base, raised, hairline: Color let textPrimary, textSecondary, textIdle: Color let accent: Color // static fallback; art may override at runtime @@ -47,13 +47,13 @@ struct Palette { } /// Fonts as size-taking closures so a point size is a call, not six stored fonts. -struct Typography { - let hero, title, section: (CGFloat) -> Font - let body, numeric: (CGFloat) -> Font +struct Typography: Sendable { + let hero, title, section: @Sendable (CGFloat) -> Font + let body, numeric: @Sendable (CGFloat) -> Font } /// Per-theme copy so idle/empty states and section headers carry personality. -struct IdleCopy { +struct IdleCopy: Sendable { let nowPlaying: String let dashboardIdle: String let dashHeaders: (now: String, quality: String, upNext: String) @@ -63,7 +63,7 @@ struct IdleCopy { /// A complete look, assembled from tokens. Value type read from the environment, /// so switching republishes one value and re-renders only the affected subtrees. -struct Theme: Equatable { +struct Theme: Equatable, Sendable { let id: ThemeID let palette: Palette let type: Typography diff --git a/project.yml b/project.yml index e3af9e4..be16df2 100644 --- a/project.yml +++ b/project.yml @@ -8,6 +8,10 @@ options: settings: base: SWIFT_VERSION: "5.0" + # Full data-race checking under the Swift 5 language mode: the codebase is + # clean against it, and keeping it on surfaces any new concurrency issue as + # a warning now (an error once we move to the Swift 6 language mode). + SWIFT_STRICT_CONCURRENCY: "complete" MACOSX_DEPLOYMENT_TARGET: "14.0" # Marketing + build numbers live here; Info.plist references them so a # release is a one-line bump. Increment CURRENT_PROJECT_VERSION for every