-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Educational Notes] Start adding educational notes for data-race safety. #79509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f1492fe
[Educational Notes] Remove complex-closure-inference.
hborla 72eab9f
[Educational Notes] Add an explanation for `sending 'x' risks causing
hborla 4353c5a
[Educational Notes] Add an explanation for the unsafe global variable
hborla cf43771
[Educational Notes] Add an explanation for captures in a `@Sendable`
hborla 0f7c991
[Educational Notes] Add an explanation for actor-isolated calls from
hborla fdd7402
[Educational Notes] Add an explanation for `sending` closure arguments.
hborla File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
# Calling an actor-isolated method from a synchronous nonisolated context | ||
|
||
Calls to actor-isolated methods from outside the actor must be done asynchronously. Otherwise, access to actor state can happen concurrently and lead to data races. These rules also apply to global actors like the main actor. | ||
|
||
For example: | ||
|
||
```swift | ||
@MainActor | ||
class MyModel { | ||
func update() { ... } | ||
} | ||
|
||
func runUpdate(model: MyModel) { | ||
model.update() | ||
} | ||
``` | ||
|
||
Building the above code produces an error about calling a main actor isolated method from outside the actor: | ||
|
||
``` | ||
| func runUpdate(model: MyModel) { | ||
| model.update() | ||
| `- error: call to main actor-isolated instance method 'update()' in a synchronous nonisolated context | ||
| } | ||
``` | ||
|
||
The `runUpdate` function doesn't specify any actor isolation, so it is `nonisolated` by default. `nonisolated` methods can be called from any concurrency domain. To prevent data races, `nonisolated` methods cannot access actor isolated state in their implementation. If `runUpdate` is called from off the main actor, calling `model.update()` could mutate main actor state at the same time as another task running on the main actor. | ||
|
||
To resolve the error, `runUpdate` has to make sure the call to `model.update()` is on the main actor. One way to do that is to add main actor isolation to the `runUpdate` function: | ||
|
||
```swift | ||
@MainActor | ||
func runUpdate(model: MyModel) { | ||
model.update() | ||
} | ||
``` | ||
|
||
Alternatively, if the `runUpdate` function is meant to be called from arbitrary concurrent contexts, create a task isolated to the main actor to call `model.update()`: | ||
|
||
```swift | ||
func runUpdate(model: MyModel) { | ||
Task { @MainActor in | ||
model.update() | ||
} | ||
} | ||
``` |
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
# Unsafe mutable global and static variables | ||
|
||
Concurrency checking prohibits mutable global and static variables that are `nonisolated` because they can be accessed from arbitrary concurrency domains at once and lead to data races. | ||
|
||
For example: | ||
|
||
```swift | ||
struct Constants { | ||
static var value = 10 | ||
} | ||
``` | ||
|
||
Building this code with complete concurrency checking will point out the unsafe static variable: | ||
|
||
``` | ||
| struct Constants { | ||
| static var value = 10 | ||
| |- error: static property 'value' is not concurrency-safe because it is nonisolated global shared mutable state | ||
| |- note: convert 'value' to a 'let' constant to make 'Sendable' shared state immutable | ||
| |- note: add '@MainActor' to make static property 'value' part of global actor 'MainActor' | ||
| `- note: disable concurrency-safety checks if accesses are protected by an external synchronization mechanism | ||
``` | ||
|
||
If the type of the variable conforms to `Sendable` and the value is never changed, a common fix is to change the `var` to a `let` to make the state immutable. Immutable state is safe to access concurrently! | ||
|
||
If you carefully access the global variable in a way that cannot cause data races, such as by wrapping all accesses in an external synchronization mechanism like a lock or a dispatch queue, you can apply `nonisolated(unsafe)` to opt out of concurrency checking: | ||
|
||
```swift | ||
nonisolated(unsafe) static var value = 10 | ||
``` | ||
|
||
Now consider a static variable with a type that does not conform to `Sendable`: | ||
|
||
```swift | ||
class MyModel { | ||
static let shared = MyModel() | ||
|
||
// mutable state | ||
} | ||
``` | ||
|
||
This code is also diagnosed under complete concurrency checking. Even though the `shared` variable is a `let` constant, the `MyModel` type is not `Sendable`, so it could have mutable stored properties. A common fix in this case is to isolate the variable to the main actor: | ||
|
||
```swift | ||
class MyModel { | ||
@MainActor | ||
static let shared = MyModel() | ||
} | ||
``` | ||
|
||
Alternatively, isolate the `MyModel` class to the main actor, which will also make the type `Sendable` because the main actor protects access to all mutable state: | ||
|
||
```swift | ||
@MainActor | ||
class MyModel { | ||
static let shared = MyModel() | ||
} | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
# Captures in a `@Sendable` closure | ||
|
||
`@Sendable` closures can be called multiple times concurrently, so any captured values must also be safe to access concurrently. To prevent data races, the compiler prevents capturing mutable values in a `@Sendable` closure. | ||
|
||
For example: | ||
|
||
```swift | ||
func callConcurrently( | ||
_ closure: @escaping @Sendable () -> Void | ||
) { ... } | ||
|
||
func capture() { | ||
var result = 0 | ||
result += 1 | ||
|
||
callConcurrently { | ||
print(result) | ||
} | ||
} | ||
``` | ||
|
||
The compiler diagnoses the capture of `result` in a `@Sendable` closure: | ||
|
||
``` | ||
| callConcurrently { | ||
| print(result) | ||
| `- error: reference to captured var 'result' in concurrently-executing code | ||
| } | ||
| } | ||
``` | ||
|
||
Because the closure is marked `@Sendable`, the implementation of `callConcurrently` can call `closure` multiple times concurrently. For example, multiple child tasks within a task group can call `closure` concurrently: | ||
|
||
```swift | ||
func callConcurrently( | ||
_ closure: @escaping @Sendable () -> Void | ||
) { | ||
Task { | ||
await withDiscardingTaskGroup { group in | ||
for _ in 0..<10 { | ||
group.addTask { | ||
closure() | ||
} | ||
} | ||
} | ||
} | ||
} | ||
``` | ||
|
||
If the type of the capture is `Sendable` and the closure only needs the value of the variable at the point of capture, resolve the error by explicitly capturing the variable by value in the closure's capture list: | ||
|
||
```swift | ||
func capture() { | ||
var result = 0 | ||
result += 1 | ||
|
||
callConcurrently { [result] in | ||
print(result) | ||
} | ||
} | ||
``` | ||
|
||
This strategy does not apply to captures with non-`Sendable` type. Consider the following example: | ||
|
||
```swift | ||
class MyModel { | ||
func log() { ... } | ||
} | ||
|
||
func capture(model: MyModel) async { | ||
callConcurrently { | ||
model.log() | ||
} | ||
} | ||
``` | ||
|
||
The compiler diagnoses the capture of `model` in a `@Sendable` closure: | ||
|
||
``` | ||
| func capture(model: MyModel) async { | ||
| callConcurrently { | ||
| model.log() | ||
| `- error: capture of 'model' with non-sendable type 'MyModel' in a '@Sendable' closure | ||
| } | ||
| } | ||
``` | ||
|
||
If a type with mutable state can be referenced concurrently, but all access to mutable state happens on the main actor, isolate the type to the main actor and mark the methods that don't access mutable state as `nonisolated`: | ||
|
||
```swift | ||
@MainActor | ||
class MyModel { | ||
nonisolated func log() { ... } | ||
} | ||
|
||
func capture(model: MyModel) async { | ||
callConcurrently { | ||
model.log() | ||
} | ||
} | ||
``` | ||
|
||
The compiler will guarantee that the implementation of `log` does not access any main actor state. | ||
|
||
If you manually ensure data-race safety, such as by using an external synchronization mechanism, you can use `nonisolated(unsafe)` to opt out of concurrency checking: | ||
|
||
```swift | ||
class MyModel { | ||
func log() { ... } | ||
} | ||
|
||
func capture(model: MyModel) async { | ||
nonisolated(unsafe) let model = model | ||
callConcurrently { | ||
model.log() | ||
} | ||
} | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
# Sending closure risks causing data races | ||
|
||
If a type does not conform to `Sendable`, the compiler enforces that each instance of that type is only accessed by one concurrency domain at a time. The compiler also prevents you from capturing values in closures that are sent to another concurrency domain if the value can be accessed from the original concurrency domain too. | ||
|
||
For example: | ||
|
||
```swift | ||
class MyModel { | ||
var count: Int = 0 | ||
|
||
func perform() { | ||
Task { | ||
self.update() | ||
} | ||
} | ||
|
||
func update() { count += 1 } | ||
} | ||
``` | ||
|
||
The compiler diagnoses the capture of `self` in the task closure: | ||
|
||
``` | ||
| class MyModel { | ||
| func perform() { | ||
| Task { | ||
| `- error: passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure | ||
| self.update() | ||
| `- note: closure captures 'self' which is accessible to code in the current task | ||
| } | ||
| } | ||
``` | ||
|
||
This code is invalid because the task that calls `perform()` runs concurrently with the task that calls `update()`. The `MyModel` type does not conform to `Sendable`, and it has unprotected mutable state that both concurrent tasks could access simultaneously. | ||
|
||
To eliminate the risk of data races, all tasks that can access the `MyModel` instance must be serialized. The easiest way to accomplish this is to isolate `MyModel` to a global actor, such as the main actor: | ||
|
||
```swift | ||
@MainActor | ||
class MyModel { | ||
func perform() { | ||
Task { | ||
self.update() | ||
} | ||
} | ||
|
||
func update() { ... } | ||
} | ||
``` | ||
|
||
This resolves the data race because the two tasks that can access the `MyModel` value must switch to the main actor to access its state and methods. | ||
|
||
The other approach to resolving the error is to ensure that only one task has access to the `MyModel` value at a time. For example: | ||
|
||
```swift | ||
class MyModel { | ||
static func perform(model: sending MyModel) { | ||
Task { | ||
model.update() | ||
} | ||
} | ||
|
||
func update() { ... } | ||
} | ||
``` | ||
|
||
This code is safe from data races because the caller of `perform` cannot access the `model` parameter again after the call. The `sending` parameter modifier indicates that the implementation of the function sends the value to a different concurrency domain, so it's no longer safe to access the value in the caller. This ensures that only one task has access to the value at a time. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.