Skip to content

Commit

Permalink
Make 'didSet' main actor. (#3206)
Browse files Browse the repository at this point in the history
* Make 'didSet' main actor.

* tests for AppStorageKey

* wip

* tests

* clean up

* Update Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift

* Update Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKeyPathKey.swift

* try working around CI problem

* Added NB

* wip

* Update MigratingTo1.11.md

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>
  • Loading branch information
mbrandonw and stephencelis authored Jul 17, 2024
1 parent a200f6a commit 54eb417
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ APIs and deprecated 1 API.
> Important: Before following this migration guide be sure you have fully migrated to the newest
> tools of version 1.10. See <doc:MigrationGuides> for more information.
* [Mutating shared state concurrently](#Mutating-shared-state-concurrently)
* [Supplying mock read-only state to previews](#Supplying-mock-read-only-state-to-previews)
* [Migrating to 1.11.2](#Migrating-to-1112)

## Mutating shared state concurrently

Version 1.10 of the Composable Architecture introduced a powerful tool for
Expand Down
27 changes: 27 additions & 0 deletions Sources/ComposableArchitecture/Internal/DispatchQueue.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import Dispatch

func mainActorASAP(execute block: @escaping @MainActor @Sendable () -> Void) {
if DispatchQueue.getSpecific(key: key) == value {
assumeMainActorIsolated {
block()
}
} else {
DispatchQueue.main.async {
block()
}
}
}

private let key: DispatchSpecificKey<UInt8> = {
let key = DispatchSpecificKey<UInt8>()
DispatchQueue.main.setSpecific(key: key, value: value)
return key
}()
private let value: UInt8 = 0

// NB: Currently we can't use 'MainActor.assumeIsolated' on CI, but we can approximate this in
// the meantime.
@MainActor(unsafe)
private func assumeMainActorIsolated(_ block: @escaping @MainActor @Sendable () -> Void) {
block()
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public protocol PersistenceReaderKey<Value> {
/// deinitialized, the `didSet` closure will no longer be invoked.
func subscribe(
initialValue: Value?,
didSet: @Sendable @escaping (_ newValue: Value?) -> Void
didSet: @escaping @Sendable (_ newValue: Value?) -> Void
) -> Shared<Value>.Subscription
}

Expand All @@ -46,7 +46,7 @@ extension PersistenceReaderKey where ID == Self {
extension PersistenceReaderKey {
public func subscribe(
initialValue: Value?,
didSet: @Sendable @escaping (_ newValue: Value?) -> Void
didSet: @escaping @Sendable (_ newValue: Value?) -> Void
) -> Shared<Value>.Subscription {
Shared.Subscription {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ extension AppStorageKey: PersistenceKey {

public func subscribe(
initialValue: Value?,
didSet: @Sendable @escaping (_ newValue: Value?) -> Void
didSet: @escaping @Sendable (_ newValue: Value?) -> Void
) -> Shared<Value>.Subscription {
let previousValue = LockIsolated(initialValue)
let userDefaultsDidChange = NotificationCenter.default.addObserver(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ extension AppStorageKeyPathKey: PersistenceKey, Hashable {

public func subscribe(
initialValue: Value?,
didSet: @Sendable @escaping (_ newValue: Value?) -> Void
didSet: @escaping @Sendable (_ newValue: Value?) -> Void
) -> Shared<Value>.Subscription {
let observer = self.store.observe(self.keyPath, options: .new) { _, change in
guard
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public final class FileStorageKey<Value: Codable & Sendable>: PersistenceKey, Se

public func subscribe(
initialValue: Value?,
didSet: @Sendable @escaping (_ newValue: Value?) -> Void
didSet: @escaping @Sendable (_ newValue: Value?) -> Void
) -> Shared<Value>.Subscription {
let cancellable = LockIsolated<AnyCancellable?>(nil)
@Sendable func setUpSources() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public struct PersistenceKeyDefault<Base: PersistenceReaderKey>: PersistenceRead

public func subscribe(
initialValue: Base.Value?,
didSet: @Sendable @escaping (Base.Value?) -> Void
didSet: @escaping @Sendable (Base.Value?) -> Void
) -> Shared<Base.Value>.Subscription {
self.base.subscribe(initialValue: initialValue, didSet: didSet)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,10 @@ final class ValueReference<Value, Persistence: PersistenceReaderKey<Value>>: Ref
self._$perceptionRegistrar.willSet(self, keyPath: \.value)
defer { self._$perceptionRegistrar.didSet(self, keyPath: \.value) }
#endif
self.lock.withLock {
self._value = value ?? initialValue
mainActorASAP {
self.lock.withLock {
self._value = value ?? initialValue
}
}
}
}
Expand Down
94 changes: 94 additions & 0 deletions Tests/ComposableArchitectureTests/AppStorageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,100 @@ final class AppStorageTests: XCTestCase {
store.setValue(2, forKey: "other-count")
XCTAssertEqual(values.value, [1])
}

@MainActor
func testUpdateStoreFromBackgroundThread() async throws {
@Dependency(\.defaultAppStorage) var store
@Shared(.appStorage("count")) var count = 0

let publisherExpectation = expectation(description: "publisher")
let cancellable = $count.publisher.sink { _ in
XCTAssertTrue(Thread.isMainThread)
publisherExpectation.fulfill()
}
defer { _ = cancellable }

await withUnsafeContinuation { continuation in
DispatchQueue.global().async { [store = UncheckedSendable(store)] in
XCTAssertFalse(Thread.isMainThread)
store.wrappedValue.setValue(1, forKey: "count")
continuation.resume()
}
}

await fulfillment(of: [publisherExpectation], timeout: 0)
}

@MainActor
func testUpdateStoreFromMainThread() async throws {
@Dependency(\.defaultAppStorage) var store
@Shared(.appStorage("count")) var count = 0
let isInStackFrame = LockIsolated(false)

let publisherExpectation = expectation(description: "publisher")
let cancellable = $count.publisher.sink { _ in
XCTAssertTrue(Thread.isMainThread)
XCTAssertTrue(isInStackFrame.value)
publisherExpectation.fulfill()
}
defer { _ = cancellable }

await withUnsafeContinuation { continuation in
XCTAssertTrue(Thread.isMainThread)
isInStackFrame.withValue { $0 = true }
store.setValue(1, forKey: "count")
isInStackFrame.withValue { $0 = false }
continuation.resume()
}

await fulfillment(of: [publisherExpectation], timeout: 0)
}

@MainActor
func testWillEnterForegroundFromBackgroundThread() async throws {
@Shared(.appStorage("count")) var count = 0

let publisherExpectation = expectation(description: "publisher")
let cancellable = $count.publisher.sink { _ in
XCTAssertTrue(Thread.isMainThread)
publisherExpectation.fulfill()
}
defer { _ = cancellable }

await withUnsafeContinuation { continuation in
DispatchQueue.global().async {
XCTAssertFalse(Thread.isMainThread)
NotificationCenter.default.post(name: willEnterForegroundNotificationName!, object: nil)
continuation.resume()
}
}

await fulfillment(of: [publisherExpectation], timeout: 0)
}

@MainActor
func testUpdateStoreFromBackgroundThread_KeyPath() async throws {
@Dependency(\.defaultAppStorage) var store
@Shared(.appStorage(\.count)) var count = 0

let publisherExpectation = expectation(description: "publisher")
publisherExpectation.expectedFulfillmentCount = 2
let cancellable = $count.publisher.sink { _ in
XCTAssertTrue(Thread.isMainThread)
publisherExpectation.fulfill()
}
defer { _ = cancellable }

await withUnsafeContinuation { continuation in
DispatchQueue.global().async { [store = UncheckedSendable(store)] in
XCTAssertFalse(Thread.isMainThread)
store.wrappedValue.count = 1
continuation.resume()
}
}

await fulfillment(of: [publisherExpectation], timeout: 0)
}
}

extension UserDefaults {
Expand Down
28 changes: 28 additions & 0 deletions Tests/ComposableArchitectureTests/FileStorageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,34 @@ final class FileStorageTests: XCTestCase {
XCTAssertEqual(count, max * (max + 1) / 2)
}
}

@MainActor
func testUpdateFileSystemFromBackgroundThread() async throws {
await withDependencies {
$0.defaultFileStorage = .fileSystem
} operation: {
try? FileManager.default.removeItem(at: .fileURL)

@Shared(.fileStorage(.fileURL)) var count = 0

let publisherExpectation = expectation(description: "publisher")
let cancellable = $count.publisher.sink { _ in
XCTAssertTrue(Thread.isMainThread)
publisherExpectation.fulfill()
}
defer { _ = cancellable }

await withUnsafeContinuation { continuation in
DispatchQueue.global().async {
XCTAssertFalse(Thread.isMainThread)
try! Data("1".utf8).write(to: .fileURL)
continuation.resume()
}
}

await fulfillment(of: [publisherExpectation], timeout: 0.1)
}
}
}

extension URL {
Expand Down

0 comments on commit 54eb417

Please sign in to comment.