I wrote up a proposal to add dynamic actor isolation checking for isolated synchronous functions under the Swift 6 language mode to catch data races at runtime caused by bugs in preconcurrency dependencies. This proposal also uses dynamic actor isolation to enable suppressing static actor isolation errors in protocol conformances for uses cases involving @preconcurrency protocols that provide a dynamic invariant that requirements are always invoked on the main actor, streamlining the common nonisolated + MainActor.assumeIsolated pattern.
This looks good overall. makes a lot of sense to automate the assume-pattern.
I think it's right to gate this behind : @preconcurrency P conformances as well like that, that looks like the right way to ease into the future while supporting old protocols with such assumptions
This would be using all the adequate SerialExecutor equality checking (and can handle custom executors with complex equality which may be interesting here if someone had a queue targeting the main queue etc...).
I would perhaps consider changing the name of the feature and flag... Mostly because this is a rather specific small piece of "dynamic isolation", that is tied both to global actors, and conformances. In general "dynamic actor isolation" sounds like it could be various different ways to get similar effects (assumeIsolated is similar after all) so it might be better to call the feature something more related to the conformances and globals that it actually speaks about? "Dynamic global actor witness isolation checking" or something like that?
Very nice! This pitch was also very timely, having just jumped through MainActor.assumeIsolated and then another hoop to implement roughly the same logic to earlier OS versions for a pre-Concurrency API conformance.
Side note: Would be very nice if assumeIsolated was @backDeployed, but I hear it's tricky; see apple/swift/pull/68378.
But that side note brings me to the question: Will the runtime checks for @preconcurrency conformances be back-deployed, and does the implementation indeed go hand-in-hand with assumeIsolated becoming back-deployed? The proposal draft says there are no deployment constraints, so I'm optimistic, but asking to make sure.
The underlying mechanism is the same and available in old versions of the runtime; the problem with backdeployment of assumeIsolated is related to @backDeployment itself, but not to the runtime issues.
I think we can consider the backdeployment issue of assumeIsolated separate from this proposal in that sense. Though the backdeployment of assume remains valuable, but blocked by some other work.
Am I understanding this correctly to mean that all actor-isolated method invocations will now incur the runtime cost of checking that they are being executed on the correct actor? Is the rationale here that because such a cost is low, and compile-time optimizations, this won’t have a meaningful performance impact? Seems reasonable to me, but might be nice to call out explicitly.
Also, just wondering, is invoking the method via a protocol the only way to get into the bad state? If so, would it be possible to have the protocol witness be a thin wrapper that just does the assumeIsolated dance and calls the underlying method?
Yes. I'll gather some concrete metrics to include in the proposal.
No, you can get into this situation when working with any preconcurrency dependencies or clients that are able to call your isolated methods. This can happen directly via public functions, anything exposed to @objc, protocol conformances, and any situation where the dependency gets a function reference to your synchronous isolated function that is later called. It's possible that there are cases where we can optimize away the assertions, e.g. perhaps if it's an internal (synchronous isolated) function, the dynamic checks could be emitted only in the witness or @objc thunks, but I worry about situations where you've suppressed @Sendable diagnostics via @preconcurrency import which hides compiler errors that enable your preconcurrency dependency to get a function reference to then call directly.
If I @preconcurrency import MyPreconcurrencyLibrary, and I try to pass a function reference to myActorIsolatedFunction as an argument to the API MyPreconcurrencyLibrary.functionThatTakesAFunctionReference(), would the type-checker not realize that this is happening in much the same way as if I passed that function reference to a strict-concurrency-checking API that took a function reference? or to ask this in another way: is "I am actor isolated" a part of the type of a function-reference? Perhaps @preconcurrency is suppressing more checks than I realize?
Sorry I wasn't very clear above. You don't actually need a @preconcurrency import to get into a situation where a dependency that does not have strict concurrency checking enabled can call your internal actor-isolated function from outside the isolation domain. Consider a library NotMyLibrary with this function:
// In NotMyLibrary
public func takesClosure(_ closure: () -> Void) { ... }
From the type checker's perspective, this code is perfectly safe. You're passing a reference to mustRunOnMainActor to a parameter of type () -> Void, which doesn't preserve the actor isolation, but the closure is not @Sendable, so it cannot actually be passed to another isolation domain! ....except in the case where NotMyLibrary does not have strict concurrency checking enabled, and it unknowingly passes that function reference off to a background thread and invokes it. I think dynamic actor isolation checking is a critical piece to allowing the ecosystem of Swift code to migrate to strict concurrency checking incrementally without forcing projects that do enable strict concurrency checking to contort their code before their dependencies have had been migrated.
Surely that is not possible, because the closure is non-escaping.
And isn't that non-escapingness why the type-checker allows the actor isolation to be stripped in the first place? Or is there another reason? Because it doesn't seem safe to allow that.
I mentioned this back in the previous thread about priorities for Swift 6:
It doesn't appear to have gotten any better. The example I gave back then, calling one @MainActor function from another:
@MainActor
func funcOne() async -> Int {
funcTwo()
}
@MainActor
func funcTwo() -> Int {
return 42
}
If I look at the Godbolt output, funcOne does a bunch of runtime calls to get MainActor.shared.unownedExecutor, and enqueues its continuation on it. That continuation does the same sequence of runtime calls to get MainActor.shared.unownedExecutor again, then calls swift_task_isCurrentExecutor to check that it has the correct isolation -- even though it's a hidden continuation that was already enqueued on that executor. And there's a bunch of retain/release traffic along the way.
In fact, if you make funcOne synchronous, the compiler will actually check the current executor twice, even though there are no suspension points between the checks.
So the implementation is in a fairly disappointing state right now. I really hoped that language integrated concurrency meant we wouldn't need these kinds of runtime checks everywhere. I still hope that things will improve because IMHO it's not ready to be enabled by default as things stand.
And if there are some source-breaking changes needed to eliminate more of these checks statically, I think it would be good to disclose and discuss them as part of the journey to Swift 6.
Fair, but @escaping does not imply @Sendable, so the type checker still allows the code even if that closure is @escaping.
No, it's safe because the closure is not @Sendable. Because the closure isn't @Sendable, it cannot be stored into a (checked) Sendable type, and if it's stored into a non-Sendable type, that value itself cannot be passed across isolation boundaries. (I don't think we need to re-consider these rules in light of the region based isolation pitch, because isolated function values should just be considered part of the actor's region and therefore never transferrable unless the isolation is preserved, in which case the function value is inherently @Sendable.) I think this is the right language rule, and I don't think we should change it just to allow more optimization of dynamic checks. Of course, that does not mean there isn't room for more optimization than what the implementation of -enable-actor-data-race-checks does today.
To be clear, this pitch does not yet have an implementation. -enable-actor-data-race-checks in its current state is a useful debugging tool, and it introduced a runtime entry point for the executor checks that has existed in the concurrency runtime from the start, setting up this pitch to not have any deployment constraints. But I'm not suggesting enabling -enable-actor-data-race-checks everywhere without additional implementation work. There's a reason why -enable-actor-data-race-checks is only mentioned in the Acknowledgments section and not in the Implementation field of the proposal draft.
