Skip to content

Commit

Permalink
Make Shared.wrappedValue setter unavailable from async and introduc…
Browse files Browse the repository at this point in the history
…e `Shared.withLock` (#3136)

* Add withValue to Shared, deprecate direct mutation.

* updates

* wip

* wip

* wip

* wip

* Available noasync

* withLock

* clean up

* wip

* wip

* Update SyncUpsListTests.swift

* wip

* wip

* wip

* wip

* wip

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>
  • Loading branch information
mbrandonw and stephencelis authored Jun 6, 2024
1 parent 9670a86 commit 1eeca17
Show file tree
Hide file tree
Showing 17 changed files with 464 additions and 70 deletions.
2 changes: 1 addition & 1 deletion Examples/SyncUps/SyncUps/AppFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ struct AppFeature {
}
Reduce { state, action in
switch action {
case let .path(.element(id, .detail(.delegate(delegateAction)))):
case let .path(.element(_, .detail(.delegate(delegateAction)))):
switch delegateAction {
case let .startMeeting(sharedSyncUp):
state.path.append(.record(RecordMeeting.State(syncUp: sharedSyncUp)))
Expand Down
4 changes: 2 additions & 2 deletions Examples/SyncUps/SyncUpsTests/AppFeatureTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ final class AppFeatureTests: XCTestCase {
AppFeature()
}

let sharedSyncUp = try XCTUnwrap($syncUps[id: syncUp.id])
let sharedSyncUp = try XCTUnwrap(Shared($syncUps[id: syncUp.id]))

await store.send(\.path.push, (id: 0, .detail(SyncUpDetail.State(syncUp: sharedSyncUp)))) {
$0.path[id: 0] = .detail(SyncUpDetail.State(syncUp: sharedSyncUp))
Expand Down Expand Up @@ -44,7 +44,7 @@ final class AppFeatureTests: XCTestCase {
AppFeature()
}

let sharedSyncUp = try XCTUnwrap($syncUps[id: syncUp.id])
let sharedSyncUp = try XCTUnwrap(Shared($syncUps[id: syncUp.id]))

await store.send(\.path.push, (id: 0, .detail(SyncUpDetail.State(syncUp: sharedSyncUp)))) {
$0.path[id: 0] = .detail(SyncUpDetail.State(syncUp: sharedSyncUp))
Expand Down
2 changes: 1 addition & 1 deletion Examples/SyncUps/SyncUpsTests/SyncUpDetailTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ final class SyncUpDetailTests: XCTestCase {
// TODO: Can this exhaustively be caught?
defer { XCTAssertEqual([], syncUps) }

let sharedSyncUp = try XCTUnwrap($syncUps[id: syncUp.id])
let sharedSyncUp = try XCTUnwrap(Shared($syncUps[id: syncUp.id]))
let store = TestStore(initialState: SyncUpDetail.State(syncUp: sharedSyncUp)) {
SyncUpDetail()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# Migrating to 1.11

Update your code to use the new ``Shared/withLock(_:)`` method for mutating shared state from
asynchronous contexts, rather than mutating the underlying wrapped value directly.

## Overview

The Composable Architecture is under constant development, and we are always looking for ways to
simplify the library, and make it more powerful. This version of the library introduced 2 new
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

Version 1.10 of the Composable Architecture introduced a powerful tool for
[sharing state](<doc:SharingState>) amongst your features. And you can mutate a piece of shared
state directly, as if it were just a normal property on a value type:

```swift
case .incrementButtonTapped:
state.count += 1
return .none
```

And if you only ever mutate shared state from a reducer, then this is completely fine to do.
However, because shared values are secretly references (that is how data is shared), it is possible
to mutate shared values from effects, which means concurrently. And prior to 1.11, it was possible
to do this directly:

```swift
case .delayedIncrementButtonTapped:
return .run { _ in
@Shared(.count) var count
count += 1
}

Now, `Shared` is `Sendable`, and is technically thread-safe in that it will not crash when writing
to it from two different threads. However, allowing direct mutation does make the value susceptible
to race conditions. If you were to perform `count += 1` from 1,000 threads, it is possible for
the final value to not be 1,000.

We wanted the [`@Shared`](<doc:Shared>) type to be as ergonomic as possible, and that is why we make
it directly mutable, but we should not be allowing these mutations to happen from asynchronous
contexts. And so now the ``Shared/wrappedValue`` setter has been marked unavailable from
asynchronous contexts, with a helpful message of how to fix:

```swift
case .delayedIncrementButtonTapped:
return .run { _ in
@Shared(.count) var count
count += 1 // ⚠️ Use '$shared.withLock' instead of mutating directly.
}
```

To fix this deprecation you can use the new ``Shared/withLock(_:)`` method on the projected value of
`@Shared`:

```swift
case .delayedIncrementButtonTapped:
return .run { _ in
@Shared(.count) var count
$count.withLock { $0 += 1 }
}
```

This locks the entire unit of work of reading the current count, incrementing it, and storing it
back in the reference.

Technically it is still possible to write code that has race conditions, such as this silly example:

```swift
let currentCount = count
$count.withLock { $0 = currentCount + 1 }
```

But there is no way to 100% prevent race conditions in code. Even actors are susceptible to problems
due to re-entrancy. To avoid problems like the above we recommend wrapping as many mutations of the
shared state as possible in a single ``Shared/withLock(_:)``. That will make sure that the full unit
of work is guarded by a lock.

## Supplying mock read-only state to previews

A new ``SharedReader/constant(_:)`` helper on ``SharedReader`` has been introduced to simplify
supplying mock data to Xcode previews. It works like SwiftUI's `Binding.constant`, but for shared
references:

```swift
#Preview {
FeatureView(
store: Store(
initialState: Feature.State(count: .constant(42))
) {
Feature()
}
)
)
```
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ as SQLite.
* [Testing tips](#Testing-tips)
* [Read-only shared state](#Read-only-shared-state)
* [Type-safe keys](#Type-safe-keys)
* [Concurrent mutations to shared state](#Concurrent-mutations-to-shared-state)
* [Shared state in pre-observation apps](#Shared-state-in-pre-observation-apps)
* [Gotchas of @Shared](#Gotchas-of-Shared)

Expand Down Expand Up @@ -269,8 +270,6 @@ case .countUpdated(let count):
If `count` changes, then `$count.publisher` emits, causing the `countUpdated` action to be sent,
causing the shared `count` to be mutated, causing `$count.publisher` to emit, and so on.



## Initialization rules

Because the state sharing tools use property wrappers there are special rules that must be followed
Expand Down Expand Up @@ -570,8 +569,8 @@ shared state in an effect, and then increments from the effect:

```swift
case .incrementButtonTapped:
return .run { [count = state.$count] _ in
count.wrappedValue += 1
return .run { [sharedCount = state.$count] _ in
sharedCount.withLock { $0 += 1 }
}
```

Expand Down Expand Up @@ -810,7 +809,7 @@ func testIncrement() async {
}
```

This will fail if you accidetally remove a `@Shared` from one of your features.
This will fail if you accidentally remove a `@Shared` from one of your features.

Further, you can enforce this pattern in your codebase by making all `@Shared` properties
`fileprivate` so that they can never be mutated outside their file scope:
Expand Down Expand Up @@ -991,6 +990,36 @@ struct FeatureView: View {
}
```

## Concurrent mutations to shared state

While the [`@Shared`](<doc:Shared>) property wrapper makes it possible to treat shared state
_mostly_ like regular state, you do have to perform some extra steps to mutate shared state from
an asynchronous context. This is because shared state is technically a reference deep down, even
though we take extra steps to make it appear value-like. And this means it's possible to mutate the
same piece of shared state from multiple threads, and hence race conditions are possible.

To mutate a piece of shared state in an isolated fashion, use the ``Shared/withLock(_:)`` method
defined on the `@Shared` projected value:

```swift
state.$count.withLock { $0 += 1 }
```

That locks the entire unit of work of reading the current count, incrementing it, and storing it
back in the reference.

Technically it is still possible to write code that has race conditions, such as this silly example:

```swift
let currentCount = state.count
state.$count.withLock { $0 = currentCount + 1 }
```

But there is no way to 100% prevent race conditions in code. Even actors are susceptible to
problems due to re-entrancy. To avoid problems like the above we recommend wrapping as many
mutations of the shared state as possible in a single ``Shared/withLock(_:)``. That will make
sure that the full unit of work is guarded by a lock.

## Gotchas of @Shared

There are a few gotchas to be aware of when using shared state in the Composable Architecture.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# ``ComposableArchitecture/Shared``

## Topics

### Creating a shared value

- ``init(_:fileID:line:)-9d3q``
- ``init(_:)``
- ``init(projectedValue:)``

### Creating a persisted value

- ``init(wrappedValue:_:fileID:line:)-512rh``
- ``init(wrappedValue:_:fileID:line:)-7a80y``
- ``init(_:fileID:line:)-8zcy1``
- ``init(_:fileID:line:)-8jqg5``
- ``init(_:fileID:line:)-gluj``

### Accessing the value

- ``wrappedValue``
- ``projectedValue``
- ``reader``
- ``subscript(dynamicMember:)-6kmzm``
- ``subscript(dynamicMember:)-22ga9``

### Isolating the value

- ``withLock(_:)``

### Unit testing the value

- ``assert(_:file:line:)``

### SwiftUI integration

- ``elements``

### Combine integration

- ``publisher``
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# ``ComposableArchitecture/SharedReader``

## Topics

### Creating a shared value

- ``init(_:)-3a38z``
- ``init(_:)-42f43``
- ``init(projectedValue:)``
- ``constant(_:)``

### Creating a persisted value

- ``init(wrappedValue:_:fileID:line:)-7q52``
- ``init(wrappedValue:_:fileID:line:)-6asu2``
- ``init(_:fileID:line:)-41rb8``
- ``init(_:fileID:line:)-3lxyf``
- ``init(_:fileID:line:)-hzp``

### Getting the value

- ``wrappedValue``
- ``projectedValue``
- ``subscript(dynamicMember:)-34wfb``

### SwiftUI integration

- ``elements``

### Combine integration

- ``publisher``
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import ComposableArchitecture
import SwiftUI

@main
struct SyncUpsApp: App {
@MainActor
static let store = Store(initialState: SyncUpsList.State()) {
SyncUpsList()
}

var body: some Scene {
WindowGroup {
NavigationStack {
SyncUpsListView(store: Self.store)
}
}
}
}
Loading

0 comments on commit 1eeca17

Please sign in to comment.