Skip to content

Commit bada612

Browse files
rjmccallrjmccall
authored
Merge pull request #2294 from ktoso/wip-executor-checkIsolated
Custom isolation checking for SerialExecutor
2 parents edde1f3 + 859dbee commit bada612

File tree

1 file changed

+207
-0
lines changed

1 file changed

+207
-0
lines changed

‎proposals/0424-custom-isolation-checking-for-serialexecutor.md

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
# Custom isolation checking for SerialExecutor
2+
3+
* Proposal: [SE-0424](0424-advanced-custom-isolation-checking-for-serialexecutor.md)
4+
* Author: [Konrad 'ktoso' Malawski](https://github.com/ktoso)
5+
* Review Manager: [John McCall](https://github.com/rjmccall)
6+
* Status: **Active Review (Feburary 22nd...March 4th, 2024)**
7+
* Implementation: [PR #71172](https://github.com/apple/swift/pull/71172)
8+
* Review: ([pitch](https://forums.swift.org/t/pitch-custom-isolation-checking-for-serialexecutor/69786))
9+
10+
## Introduction
11+
12+
[SE-0392 (Custom Actor Executors)](https://github.com/apple/swift-evolution/blob/main/proposals/0392-custom-actor-executors.md) added support for custom actor executors, but its support is incomplete. Safety checks like [`Actor.assumeIsolated`](https://developer.apple.com/documentation/swift/actor/assumeisolated(_:file:line:)) work correctly when code is running on the actor through a task, but they don't work when code is scheduled to run on the actor's executor through some other mechanism. For example, if an actor uses a serial `DispatchQueue` as its executor, a function dispatched _directly_ to the queue with DispatchQueue.async cannot use `assumeIsolated` to assert that the actor is currently isolated. This proposal fixes this by allowing custom actor executors to provide their own logic for these safety checks.
13+
14+
## Motivation
15+
16+
The Swift concurrency runtime dynamically tracks the current executor of a running task in thread-local storage. To run code on behalf of a task, an executor must call into the runtime, and the runtime will set up the tracking appropriately. APIs like `assertIsolated` and `assumeIsolated` are built on top of that functionality and perform their checks by comparing the expected executor with the current executor tracked by the runtime. If the current thread is not running a task, the runtime treats it as if it were running a non-isolated function, and the comparison will fail.
17+
18+
This logic is not sufficient to handle the situation in which code is running on an actor's serial executor, but the code is not associated with a task. Swift's default actor executors currently do not provide any way to enqueue work on them that is not associated with a task, so this situation does not apply to them. However, many custom executors do provide other APIs for enqueuing work, such as the `async` method on `DispatchSerialQueue`. These APIs are not required to inform the Swift concurrency runtime before running the code. As a result, the runtime will be unaware that the current thread is associated with an actor's executor, and checks like `assumeIsolated` will fail. This is undesirable because, as long as the executor still acts like a serial executor for any non-task code it runs this way, the code will still be effectively actor-isolated: no code that accesses the actor's isolated state can run concurrently with it.
19+
20+
The following example demonstrates such a situation:
21+
22+
```swift
23+
import Dispatch
24+
25+
actor Caplin {
26+
let queue: DispatchSerialQueue(label: "CoolQueue")
27+
28+
var num: Int // actor isolated state
29+
30+
// use the queue as this actor's `SerialExecutor`
31+
nonisolated var unownedExecutor: UnownedSerialExecutor {
32+
queue.asUnownedSerialExecutor()
33+
}
34+
35+
nonisolated func connect() {
36+
queue.async {
37+
// guaranteed to execute on `queue`
38+
// which is the same as self's serial executor
39+
queue.assertIsolated() // CRASH: Incorrect actor executor assumption
40+
self.assumeIsolated { // CRASH: Incorrect actor executor assumption
41+
num += 1
42+
}
43+
}
44+
}
45+
}
46+
```
47+
48+
Even though the code is executing on the correct Dispatch**Serial**Queue, the assertions trigger and we're left unable to access the actor's state, even though isolation-wise it would be safe and correct to do so.
49+
50+
Being able to assert isolation for non-task code this way is important enough that the Swift runtime actually already has a special case for it: even if the current thread is not running a task, isolation checking will succeed if the target actor is the `MainActor` and the current thread is the *main thread*. This problem is more general than the main actor, however; it exists for all kinds of threads which may be used as actor executors. The most important example of this is `DispatchSerialQueue`, especially because it is so commonly used in pre-concurrency code bases to provide actor-like isolation. Allowing types like `DispatchSerialQueue` to hook into isolation checking makes it much easier to gradually migrate code to actors: if an actor uses a queue as its executor, existing code that uses the queue don't have to be completely rewritten in order to access the actor's state.
51+
52+
One way to think of this proposal is that gives all `SerialExecutor`s the power to provide a "fallback" check like this, rather than keeping it special-cased to `MainActor`.
53+
54+
## Proposed solution
55+
56+
We propose to add a new last-resort mechanism to executor comparison, which will be used by all the isolation-checking APIs in the concurrency library.
57+
58+
This will be done by providing a new `checkIsolation()` protocol requirement on `SerialExecutor`:
59+
60+
```swift
61+
protocol SerialExecutor: Executor {
62+
// ...
63+
64+
/// Invoked as last resort when the Swift concurrency runtime is performing an isolation
65+
/// assertion and could not confirm that the current execution context belongs to the
66+
/// expected executor.
67+
///
68+
/// This function MUST crash the program with a fatal error if it is unable
69+
/// to prove that this thread can currently be safely treated as isolated
70+
/// to this ``SerialExecutor``. That is, if a synchronous function calls
71+
/// this method, and the method does not crash with a fatal error,
72+
/// then the execution of the entire function must be well-ordered
73+
/// with any other job enqueued on this executor, as if it were part of
74+
/// a job itself.
75+
///
76+
/// A default implementation is provided that unconditionally causes a fatal error.
77+
func checkIsolation()
78+
}
79+
80+
extension SerialExecutor {
81+
public func checkIsolation() {
82+
fatalError("Incorrect actor executor assumption, expected: \(self)")
83+
}
84+
}
85+
```
86+
87+
## Detailed design
88+
89+
This proposal adds another customization point to the Swift concurrency runtime that hooks into isolation context comparison mechanisms used by `assertIsolated`, `preconditionIsolated`, and `assumeIsolated`, as well as any implicitly injected assertions used in `@preconcurrency` code.
90+
91+
### Extended executor comparison mechanism
92+
93+
With this proposal, the logic for checking if the current executor is the same as an expected executor changes, and can be expressed using the following pseudo-code:
94+
95+
```swift
96+
// !!!! PSEUDO-CODE !!!! Simplified for readability.
97+
98+
let current = Task.current.executor
99+
100+
guard let current else {
101+
// no current executor, last effort check performed by the expected executor:
102+
expected.checkIsolated()
103+
104+
// e.g. MainActor:
105+
// MainActorExecutor.checkIsolated() {
106+
// guard Thread.isMain else { fatalError("Expected main thread!")
107+
// return // ok!
108+
// }
109+
}
110+
111+
if isSameSerialExecutor(current, expected) {
112+
// comparison takes into account "complex equality" as introduced by 'SE-0392
113+
return // ok!
114+
} else {
115+
// executor comparisons failed...
116+
117+
// give the expected executor a last chance to check isolation by itself:
118+
expected.checkIsolated()
119+
120+
// as the default implementation of checkIsolated is to unconditionally crash,
121+
// this call usually will result in crashing -- as expected.
122+
}
123+
124+
return // ok, it seems the expected executor was able to prove isolation
125+
```
126+
127+
This pseudo code snippet explains the flow of the executor comparisons. There are two situations in which the new `checkIsolated` method may be invoked: when there is no current executor present, or if all other comparisons have failed.
128+
For more details on the executor comparison logic, you can refer to [SE-0392: Custom Actor Executors](https://github.com/apple/swift-evolution/blob/main/proposals/0392-custom-actor-executors.md).
129+
130+
Specific use-cases of this API include `DispatchSerialQueue`, which would be able to implement the requirement as follows:
131+
132+
```swift
133+
// Dispatch
134+
135+
extension DispatchSerialQueue {
136+
public func checkIsolated(message: String) {
137+
dispatchPrecondition(condition: .onQueue(self)) // existing Dispatch API
138+
}
139+
}
140+
```
141+
142+
An executor that wishes to take advantage of this proposal will need to have some mechanism to identity its active worker thread. If that's not possible or desired, the executor should leave the default implementation (that unconditionally crashes) in place.
143+
144+
### Impact on async code and isolation assumtions
145+
146+
The `assumeIsolated(_:file:line:)` APIs purposefully only accept a **synchronous** closure. This is correct, and it remains correct with these proposed additions. An isolation check on an executor ensures that any actor using the executor is synchronously isolated, and the closure provided to `assumeIsolated` will execute prior to any possible async suspension. This is what makes it safe to access actor-isolated state within the closure.
147+
148+
This means that the following code snippet, while a bit unusual remains correct isolation-wise:
149+
150+
```swift
151+
actor Worker {
152+
var number: Int
153+
154+
nonisolated func canOnlyCallMeWhileIsolatedOnThisInstance() -> Int {
155+
self.preconditionIsolated("This method must be called while isolated to \(self)")
156+
157+
return self.assumeIsolated { // () throws -> Int
158+
// suspensions are not allowed in this closure.
159+
160+
self.number // we are guaranteed to be isolated on this actor; read is safe
161+
}
162+
}
163+
164+
```
165+
166+
As such, there is no negative impact on the correctness of these APIs.
167+
168+
Asynchronous functions should not use dyanamic isolation checking. Isolation checking is useful in synchronous functions because they naturally inherit execution properties like their caller's isolation without disturbing it. A synchronous function may be formally non-isolated and yet actually run in an isolated context dynamically. This is not true for asynchronous functions, which switch to their formal isolation on entry without regard to their caller's isolation. If an asynchronous function is not formally isolated to an actor, its execution will never be dynamically in an isolated context, so there's no point in checking for it.
169+
170+
## Future directions
171+
172+
### Introduce `globalMainExecutor` global property and utilize `checkIsolated` on it
173+
174+
This proposal also paves the way to clean up this hard-coded aspect of the runtime, and it would be possible to change these heurystics to instead invoke the `checkIsolation()` method on a "main actor executor" SerialExecutor reference if it were available.
175+
176+
This proposal does not introduce a `globalMainActorExecutor`, however, similar how how [SE-0417: Task ExecutorPreference](https://github.com/apple/swift-evolution/blob/main/proposals/0417-task-executor-preference.md) introduced a:
177+
178+
```swift
179+
nonisolated(unsafe)
180+
public var globalConcurrentExecutor: any TaskExecutor { get }
181+
```
182+
183+
the same could be done to the MainActor's executor:
184+
185+
```swift
186+
nonisolated(unsafe)
187+
public var globalMainExecutor: any SerialExecutor { get }
188+
```
189+
190+
The custom heurystics that are today part of the Swift Concurrency runtime to detect the "main thread" and "main actor executor", could instead be delegated to this global property, and function correctly even if the MainActor's executor is NOT using the main thread (which can happen on some platforms):
191+
192+
```swift
193+
// concurrency runtime pseudo-code
194+
if expectedExecutor.isMainActor() {
195+
expectedExecutor.checkIsolated(message: message)
196+
}
197+
```
198+
199+
This would allow the isolation model to support different kinds of main executor and properly assert their isolation, using custom logic, rather than hardcoding the main thread assumptions into the Swift runtime.
200+
201+
## Alternatives considered
202+
203+
### Do not provide customization points, and just hardcode DispatchQueue handling
204+
205+
Alternatively, we could hardcode detecting dispatch queues and triggering `dispatchPrecondition` from within the Swift runtime.
206+
207+
This is not a good direction though, as our goal is to have the concurrency runtime be less attached to Dispatch and allow Swift to handle each and every execution environment equally well. As such, introducing necessary hooks as official and public API is the way to go here.

0 commit comments

Comments
 (0)