crossmate

A collaborative crossword app for iOS
Log | Files | Refs | LICENSE

commit 8b65e9e554b3fa9a74cd2bef642989491469bf65
parent b5a2543953234d63eae72ea805d8dbc8106507db
Author: Michael Camilleri <[email protected]>
Date:   Sun, 26 Apr 2026 19:43:30 +0900

Remove unnecessary lock

Diffstat:
MCrossmate/Persistence/GameMutator.swift | 4++--
MCrossmate/Persistence/GameStore.swift | 2+-
MCrossmate/Services/AppServices.swift | 2--
MCrossmate/Sync/AuthorIdentity.swift | 26++++++++++----------------
MCrossmate/Sync/SyncEngine.swift | 9---------
MTests/Unit/Sync/AuthorIdentityTests.swift | 24+++++-------------------
6 files changed, 18 insertions(+), 49 deletions(-)

diff --git a/Crossmate/Persistence/GameMutator.swift b/Crossmate/Persistence/GameMutator.swift @@ -16,7 +16,7 @@ final class GameMutator { private let game: Game let gameID: UUID private let moveBuffer: MoveBuffer? - private let authorIDProvider: (@Sendable () -> String?)? + private let authorIDProvider: (@MainActor () -> String?)? /// `true` when the current user owns the CloudKit zone for this game. let isOwned: Bool @@ -33,7 +33,7 @@ final class GameMutator { game: Game, gameID: UUID, moveBuffer: MoveBuffer?, - authorIDProvider: (@Sendable () -> String?)? = nil, + authorIDProvider: (@MainActor () -> String?)? = nil, isOwned: Bool = true, isShared: Bool = false, isAccessRevoked: Bool = false diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift @@ -95,7 +95,7 @@ final class GameStore { /// Provides the current iCloud author ID at move-emit time. Set by /// `AppServices` after `AuthorIdentity` is initialised. @ObservationIgnored - var authorIDProvider: (@Sendable () -> String?)? + var authorIDProvider: (@MainActor () -> String?)? /// Called when a new game's `ckRecordName` is ready to push. Wired to /// `SyncEngine.enqueueGame` by `AppServices`. diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift @@ -95,7 +95,6 @@ final class AppServices { await syncEngine.setOnAccountChange { [weak self] in guard let self else { return } await self.identity.refresh(using: self.ckContainer) - await self.syncEngine.setLocalAuthorID(self.identity.currentID) } await syncEngine.setOnGameAccessRevoked { [store] gameID in @@ -111,7 +110,6 @@ final class AppServices { // Fetch identity before starting engines so first moves get an authorID. await identity.refresh(using: ckContainer) - await syncEngine.setLocalAuthorID(identity.currentID) // NameBroadcaster fans out name changes to all shared/joined games. nameBroadcaster = NameBroadcaster( diff --git a/Crossmate/Sync/AuthorIdentity.swift b/Crossmate/Sync/AuthorIdentity.swift @@ -1,29 +1,23 @@ import CloudKit import Foundation -import os -/// Thread-safe cache for the current iCloud user's record name. Populated -/// asynchronously at app start and refreshed on account-change events from -/// `CKSyncEngine`. The synchronous `currentID` getter lets `GameMutator` -/// stamp moves without an `await` at the call site. -final class AuthorIdentity: Sendable { - private let storage: OSAllocatedUnfairLock<String?> +/// Cache for the current iCloud user's record name. Populated asynchronously +/// at app start and refreshed on account-change events from `CKSyncEngine`. +/// The synchronous `currentID` getter lets `GameMutator` stamp moves without +/// an `await` at the call site. +@MainActor +final class AuthorIdentity { + private(set) var currentID: String? - init() { - storage = OSAllocatedUnfairLock(initialState: nil) - } + init() {} /// Injects a known ID without hitting CloudKit — use in unit tests only. init(testing fixedID: String) { - storage = OSAllocatedUnfairLock(initialState: fixedID) - } - - var currentID: String? { - storage.withLock { $0 } + currentID = fixedID } func refresh(using container: CKContainer) async { guard let recordID = try? await container.userRecordID() else { return } - storage.withLock { $0 = recordID.recordName } + currentID = recordID.recordName } } diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift @@ -1,7 +1,6 @@ import CloudKit import CoreData import Foundation -import os.lock import SwiftUI extension EnvironmentValues { @@ -42,10 +41,6 @@ actor SyncEngine { private var onSnapshotsSaved: (@MainActor @Sendable ([String]) async -> Void)? private var tracer: (@MainActor @Sendable (String) -> Void)? - // Cached local identity — written from the main actor, read from - // nonisolated buildRecord via OSAllocatedUnfairLock. - private let cachedLocalAuthorID = OSAllocatedUnfairLock<String?>(initialState: nil) - func setTracer(_ t: @MainActor @Sendable @escaping (String) -> Void) { tracer = t } @@ -66,10 +61,6 @@ actor SyncEngine { onSnapshotsSaved = cb } - func setLocalAuthorID(_ id: String?) { - cachedLocalAuthorID.withLock { $0 = id } - } - init(container: CKContainer, persistence: PersistenceController) { self.container = container self.persistence = persistence diff --git a/Tests/Unit/Sync/AuthorIdentityTests.swift b/Tests/Unit/Sync/AuthorIdentityTests.swift @@ -5,6 +5,7 @@ import Testing @testable import Crossmate @Suite("AuthorIdentity") +@MainActor struct AuthorIdentityTests { @Test("Initial state returns nil before any refresh") @@ -13,25 +14,10 @@ struct AuthorIdentityTests { #expect(identity.currentID == nil) } - @Test("currentID is readable synchronously from any context") - func synchronousRead() async { - let identity = AuthorIdentity() - // Simulate a refresh that writes a known value. - // We bypass the real CKContainer by writing directly via the actor. - // Since AuthorIdentity uses a lock, concurrent reads are safe. - let id = identity.currentID - #expect(id == nil) - } - - @Test("Concurrent reads while value is nil do not crash") - func concurrentReadsAreSafe() async { - let identity = AuthorIdentity() - await withTaskGroup(of: String?.self) { group in - for _ in 0..<50 { - group.addTask { identity.currentID } - } - for await _ in group { } - } + @Test("Testing initializer seeds currentID") + func testingInitializerSeedsValue() { + let identity = AuthorIdentity(testing: "_abc") + #expect(identity.currentID == "_abc") } @Test("refresh with unavailable account leaves currentID nil")