Skip to content

Commit edde1f3

Browse files
hborlaxedinamartini51airspeedswift
authored
Dynamic actor isolation enforcement take 2 (#2333)
* Add a proposal for dynamic actor isolation checking. * Clarify scope of dynamic actor isolation checks * Clarify when preconcurrency witness thunks are applicable. * Add an alternative considered section for always emitting dynamic checks upon entry to synchronous isolated functions. * Make the title more specific. * Fix markup issue by normalizing indentation (#4) Mixing hard tabs and spaces caused the paragraph that starts with "The witness checker diagnostic will be suppressed" to be (incorrectly) rendered as a code listing. * Update and rename NNNN-dynamic-actor-isolation.md to 0423-dynamic-actor-isolation.md Assign review # and manager --------- Co-authored-by: Pavel Yaskevich <[email protected]> Co-authored-by: Alex Martini <[email protected]> Co-authored-by: Ben Cohen <[email protected]>
1 parent 939358e commit edde1f3

File tree

1 file changed

+152
-0
lines changed

1 file changed

+152
-0
lines changed

‎proposals/0423-dynamic-actor-isolation.md

+152
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
# Dynamic actor isolation enforcement from non-strict-concurrency contexts
2+
3+
* Proposal: [SE-0423](NNNN-dynamic-actor-isolation.md)
4+
* Authors: [Holly Borla](https://github.com/hborla), [Pavel Yaskevich](https://github.com/xedin)
5+
* Review Manager: [Ben Cohen](https://github.com/airspeedswift)
6+
* Status: **Active Review (Feb 20 - Mar 1 2024)**
7+
* Upcoming Feature Flag: `DynamicActorIsolation`
8+
* Review: ([pitch](https://forums.swift.org/t/pitch-dynamic-actor-isolation-enforcement/68354))
9+
* Implementation: [apple/swift#70867](https://github.com/apple/swift/pull/70867), [apple/swift#71261](https://github.com/apple/swift/pull/71261), [apple/swift-syntax#2419](https://github.com/apple/swift-syntax/pull/2419)
10+
11+
## Introduction
12+
13+
Many Swift programs need to interoperate with frameworks written in C/C++/Objective-C whose implementations cannot participate in static data race safety. Similarly, many Swift programs have dependencies that have not yet adopted strict concurrency checking. A `@preconcurrency import` statement downgrades concurrency-related error messages that the programmer cannot resolve because the fundamental issue is in one of the dependencies. To strengthen Swift's data-race safety guarantees while working with preconcurrency dependencies, this proposals adds actor isolation checking at runtime for synchronous isolated functions.
14+
15+
## Motivation
16+
17+
The ecosystem of Swift libraries has a vast surface area of APIs that predate strict concurrency checking, relying on carefully calling APIs from the appropriate thread or dispatch queue to avoid data races. Migrating all of these libraries to strict concurrency checking will happen incrementally, motivating [SE-0337: Incremental migration to concurrency checking](https://github.com/apple/swift-evolution/blob/main/proposals/0337-support-incremental-migration-to-concurrency-checking.md) which introduced the `@preconcurrency import` statement to suppress concurrency warnings from APIs that programmers do not control.
18+
19+
If an actor isolation violation exists in the implementation of a preconcurrency library, the bug is only surfaced to clients as hard-to-debug data races on isolated state. `@preconcurrency` also does not apply to protocol conformances; there is no way to suppress concurrency diagnostics when conforming to a protocol from a preconcurrency library. This is unfortunate, because it's common for protocols to have a dynamic invariant that all requirements are called on the main thread or a specific dispatch queue provided by the client.
20+
21+
For example, consider the following protocol in a library called `NotMyLibrary`, which provides a guarantee that its requirements are always called from the main thread:
22+
23+
```swift
24+
public protocol ViewDelegateProtocol {
25+
func respondToUIEvent()
26+
}
27+
```
28+
29+
and a client of `NotMyLibrary` that contains a conformance to `ViewDelegateProtocol`:
30+
31+
```swift
32+
import NotMyLibrary
33+
34+
@MainActor
35+
class MyViewController: ViewDelegateProtocol {
36+
func respondToUIEvent() { // error: @MainActor function cannot satisfy a nonisolated requirement
37+
// implementation...
38+
}
39+
}
40+
```
41+
42+
The above code is invalid because `MyViewController.respondToUIEvent()` is `@MainActor`-isolated, but it satisfies a `nonisolated` protocol requirement that can be called from generic code off the main actor. If the library provides a dynamic guarantee that the requirement is always called on the main actor, a sensible workaround is to resort to dynamic actor isolation checking by marking the function as `nonisolated` and wrapping the implementation in `MainActor.assumeIsolated`:
43+
44+
```swift
45+
import NotMyLibrary
46+
47+
@MainActor
48+
class MyViewController: ViewDelegateProtocol {
49+
nonisolated func respondToUIEvent() {
50+
MainActor.assumeIsolated {
51+
// implementation...
52+
}
53+
}
54+
}
55+
```
56+
57+
With this workaround, the programmer must annotate every witness with `nonisolated` and wrap the implementation in `MainActor.assumeIsolated`. More importantly, the programmer loses static data-race safety in their own code, because internal callers of `respondToUIEvent()` are free to invoke it from any isolation domain without compiler errors.
58+
59+
## Proposed solution
60+
61+
This proposal adds dynamic actor isolation checking to:
62+
63+
- Witnesses of synchronous `nonisolated` protocol requirements when the witness is isolated and the protocol conformance is annotated as `@preconcurrency`. For example:
64+
65+
If `respondToUIEvent` is a witness to a synchronous `nonisolated` protocol requirement, the protocol conformance error can be suppressed using a `@preconcurrency` annotation on the protocol to indicate that the protocol itself predates concurrency:
66+
67+
```swift
68+
import NotMyLibrary
69+
70+
@MainActor
71+
class MyViewController: @preconcurrency ViewDelegateProtocol {
72+
func respondToUIEvent() {
73+
// implementation...
74+
}
75+
}
76+
```
77+
78+
The witness checker diagnostic will be suppressed, the actor isolation assertion will fail if `respondToUIEvent()` is called inside `NonMyLibrary` from off the main actor, and the compiler will continue to emit diagnostics inside the module when called from off the main actor.
79+
80+
These dynamic checks apply to any situation where a synchronous `nonisolated` requirement is implemented by an isolated method, including synchronous actor methods.
81+
82+
- `@objc` thunks of synchronous actor-isolated members of classes.
83+
84+
Similarly to the previous case if a class or its individual synchronous members are actor-isolated and marked as either `@objc` or `@objcMembers`, the thunks, synthesized by the compiler to make them available from Objective-C, would have a new precondition check to make sure that use always happens on the right actor.
85+
86+
- Synchronous actor isolated function values passed to APIs that erase actor isolation and haven't yet adopted strict concurrency checking.
87+
88+
When API comes from a module that doesn't have strict concurrency checking enabled it's possible that it could introduce actor isolation violations that would not be surfaced to a client. In such cases actor isolation erasure should be handled defensively by introducing a runtime check at each position for granular protection.
89+
90+
```swift
91+
@MainActor
92+
func updateUI(view: MyViewController) {
93+
NotMyLibrary.track(view.renderToUIEvent)
94+
}
95+
```
96+
97+
The use of `track` here would be considered unsafe if it accepts a synchronous nonisolated function type due to loss of `@MainActor` from `renderToUIEvent` and compiler would transform the call site into a function equivalent of:
98+
99+
```swift
100+
@MainActor
101+
func updateUI(view: MyViewController) {
102+
NotMyLibrary.track({
103+
MainActor.assumeIsolated {
104+
view.renderToUIEvent()
105+
}
106+
})
107+
}
108+
```
109+
110+
111+
These are the most common circumstances when loosing actor isolation could be problematic and restricting runtime checking to them significantly limits negative performance impact of the new checks. The strategy of only emitting runtime checks when there’s potential for the function to be called from unchecked code is desirable, because it means the dynamic checks will be eliminated as more of the Swift ecosystem transitions to Swift 6.
112+
113+
114+
## Detailed design
115+
116+
### Runtime actor isolation checking
117+
118+
For all of the situations described in the previous section the compiler will emit a runtime check to assert that the current executor matches the expected executor of the isolated actor. Calling an isolated synchronous function from outside the isolation domain will result in a runtime error that halts program execution.
119+
120+
Runtime checking for actor isolation is not necessary for `async` functions, because switching to the callee's actor is always performed by the callee. `async` functions cannot be unsafely called from non-Swift code because they are not available directly in C/C++/Objective-C.
121+
122+
### `@preconcurrency` conformances
123+
124+
A `@preconcurrency` protocol conformance is scoped to the implementation of the protocol requirements in the conforming type. A `@preconcurrency` conformance can be written at the primary declartaion or in an extension, and witness checker diagnostics about actor isolation will be suppressed. Like other `@preconcurrency` annotations, if no diagnotsics are suppressed, a warning will be emitted at the `@preconcurrency` annotation stating that the annotation has no effect and it should be removed.
125+
126+
## Source compatibility
127+
128+
Dynamic actor isolation checking can introduce new runtime assertions for existing programs. Therefore, dynamic actor isolation is only performed for synchronous functions that are witnesses to an explicitly annotated `@preconcurrency` protocol conformance, or that are compiled under the Swift 6 language mode.
129+
130+
## ABI compatibility
131+
132+
This proposal has no impact on ABI compatibility of existing code. There are runtime implications for code that explicitly adopts this feature; see the following section.
133+
134+
## Implications on adoption
135+
136+
This feature can be freely adopted and un-adopted in source code with no deployment constraints and without affecting source or ABI compatibility. However, as noted in the Source compatibility section, adoption of this feature has runtime implications, because actor-isolated code called incorrectly from preconcurrency code will crash instead of race.
137+
138+
## Alternatives considered
139+
140+
### Always emit dynamic checks upon entry to synchronous isolated functions
141+
142+
A previous iteration of this proposal specified that dynamic actor isolation checks are always emitted upon entry to a synchronous isolated function. This approach is foolproof; there's little possiblity for missing a dynamic check for code that can be called from another module that does not have strict concurrency checking at compile time. However, the major downside of this approach is that code will be paying the price of runtime overhead for actor isolation checking even when actor isolation is fully enforced at compile time in Swift 6.
143+
144+
The current approach in this proposal has a very desirable property of eliminated more runtime overhead as more of the Swift ecosystem transitions to Swift 6 at the cost of introducing the potential for missing dynamic checks where synchronous functions can be called from not-statically-checked code. We believe this is the right tradeoff for the long term arc of data race safety in Swift 6 and beyond, but it may require more special cases when we discover code patterns that are not covered by the specific set of rules in this proposal.
145+
146+
### `@preconcurrency(unsafe)` to downgrade dynamic actor isolation violations to warnings
147+
148+
If adoption of this feature exposes a bug in existing binaries because actor isolated code from outside the actor, a `@preconcurrency(unsafe)` annotation (or similar) could be provided to downgrade assertion failures to warnings. However, it's not clear whether allowing a known data race exhibited at runtime is the right approach to solving such a problem.
149+
150+
## Acknowledgments
151+
152+
Thank you to Doug Gregor for implementing the existing dynamic actor isolation checking gated behind `-enable-actor-data-race-checks`.

0 commit comments

Comments
 (0)