Incorrect actor isolation assumption across Swift 5/6 module boundary leads to `dispatch_assert_queue` crashes

[NachoSoto]

Reproduction

Consider the following code, split in 2 modules:

// Module 1, compiled with Swift 5 and no strict concurrency checking:
public class A {
  public init() {}

  public func f(_ value: @escaping () -> Void) {
    DispatchQueue.global().async { value() }
  }
}

// Module 2, compiled with Swift 6:
import Module1

@MainActor
func g() {
  A().f {
    // This is assumed to be @MainActor isolated by default, leading to `dispatch_assert_queue` failed assertion
  }
}

Expected behavior

The code compiles with no warnings (because the closure passed to f is valid based on the definition), but it crashes at runtime due to it being inferred as @MainActor, and executing outside of it.

The closure shouldn't become @MainActor isolated implicitly just because it's called from a @MainActor method.

Environment

swift-driver version: 1.112.3 Apple Swift version 6.0 (swiftlang-6.0.0.6.8 clang-1600.0.23.1)
Target: arm64-apple-macosx14.0

Additional information

No response

[NachoSoto]

cc @hborla

[mattmassicotte]

For what it's worth, this exact situation is covered here in the migration guide:

https://www.swift.org/migration/documentation/swift-6-concurrency-migration-guide/incrementaladoption#Unmarked-Sendable-Closures

[hborla]

Yes, this is a deliberate language design choice. I was leaving this open to try to dig up documentation for why, and if it doesn't exist, I'll write it up.

[NachoSoto]

This code will compile without issue but crash at runtime.

So you know that this would lead to a runtime crash and still decide to not produce a warning or error? 🫣

[hborla]

So you know that this would lead to a runtime crash and still decide to not produce a warning or error? 🫣

I greatly appreciate your constructive feedback and your informative bug reports, but this sort of response isn't helpful. Please at least give me a chance to write up why the design decision was made.

[NachoSoto]

Apologies, I had understood that this was by design and was very confused about this decision.

Per https://www.swift.org/migration/documentation/swift-6-concurrency-migration-guide/incrementaladoption/#Unmarked-Sendable-Closures, adding @Sendable in would fix this, but it's not feasible to go through the entire codebase and find instances where this is needed to prevent crashes if the compiler can't at last warn us about it.

IMO, considering that Swift still doesn't have a way to silence individual warnings (and I'd like to continue compiling with warnings as errors), I would love for this behavior to not crash and also not produce a warning.

[realityworks]

So just to clarify, there is no warning this could happen? It could happen with any app that links with a Swift 6 module?

[Jon889]

If it's known that it will cause a runtime crash (which it seems to be) then it really should be an error that can be resolved by marking the closure as @Sendable as the migration doc says. Without an error it's going to be very hard to find where this occurs in any app other than sample or toy apps.

it makes incremental adoption difficult because this will introduce crashes in code that hasn't changed. So there would be no way to incrementally adopt and test an app. The whole app will need to be tested when just one module is updated to Swift 6. That's quite a big ask for commercial apps.

[hborla]

Perhaps my comment was misunderstood. The compiler does not and cannot know the code will crash at runtime, because it cannot see across module boundaries into the implementation of the function that contains the violation. My comment here:

Yes, this is a deliberate language design choice.

Means that the design choice to assume that non-Sendable closures don't leave the isolation domain they're formed in is deliberate, and it's valuable for eliminating the annotation burden in the long run. I want to elaborate on why, but that'll take me some time to articulate.

There is no need to pile onto this issue. I understand that people are struggling with runtime crashes that they can't predict, and I'm listening to the feedback. I'm not saying that no improvement is possible, but the solution is not as obvious as you all are making it out to be.

[hborla]

I would love for this behavior to not crash and also not produce a warning.

You can disable the dynamic actor isolation checking with -disable-dynamic-actor-isolation.

[NachoSoto]

Thanks @hborla. I understand that this is by no means a simple problem with a simple solution, but I look forward to seeing what you all come up with 🙏🏻

[NachoSoto]

You can disable the dynamic actor isolation checking with -disable-dynamic-actor-isolation.

Is there a way to pass that through SPM?

[freak4pc]

You can disable the dynamic actor isolation checking with -disable-dynamic-actor-isolation.

Is there a way to pass that through SPM?

I think something like this would possibly work?

.target(
    name: "...",
    swiftSettings: [.unsafeFlags(["-Xfrontend", "-disable-dynamic-actor-isolation"])]
)

[nmggithub]

You can disable the dynamic actor isolation checking with -disable-dynamic-actor-isolation.

Is there a way to pass that through SPM?

I think something like this would possibly work?

.target(
    name: "...",
    swiftSettings: [.unsafeFlags(["-Xfrontend", "-disable-dynamic-actor-isolation"])]
)

To my knowledge, this would disallow the package from being used as a versioned dependency in other packages. It could be used as a revision dependency though: https://forums.swift.org/t/swift-package-manager-doesnt-check-for-unsafe-flags-when-the-dependency-uses-revision/73281

[mattmassicotte]

I just want to make absolutely sure that everyone reading here understands that by disabling this runtime check you are explicitly opting into isolation violations. Stuff is going to get accessed on the the wrong thread.

[NachoSoto]

Should the compiler not infer actor isolation in this closure? I don't understand why it's:

• Implicitly becoming @MainActor
• Allowed to be passed as a parameter to a function that doesn't take a @MainActor closure

And to make it worse:

• Crash with dispatch_queue_assert instead of producing a runtime warning.

[mattmassicotte]

Should the compiler not infer actor isolation in this closure?

The compiler is currently following the rules as the language defines them. I think this issue here is the implication of how these rules interact with code that was compiled without them.

[krzyzanowskim]

The compiler is currently following the rules as the language defines them.

@mattmassicotte do you mind linking where the implicit behavior is documented, for future reference? (it's not easy to navigate Swift Concurrency rules)

[NachoSoto]

Just ran into another example of this that wasn't caught at compile time, but crashed at runtime:

If I hadn't noticed this, we could have shipped an instant crash-loop to our users, due to an issue that should have been detected at compile time, but instead it's a runtime dispatch_assert_queue crash.

This is completely unacceptable and I truly hope Swift 6 does not ship in this state.

[mattmassicotte]

The compiler is currently following the rules as the language defines them.

@mattmassicotte do you mind linking where the implicit behavior is documented, for future reference? (it's not easy to navigate Swift Concurrency rules)

I think the most relevant proposal is SE-0302: Sendable and @Sendable closures.

[hborla]

do you mind linking where the implicit behavior is documented, for future reference? (it's not easy to navigate Swift Concurrency rules)

I added documentation for this behavior in the migration guide here: https://www.swift.org/migration/documentation/swift-6-concurrency-migration-guide/dataracesafety#Function-Types

I think the text here can still be improved, but the philosophy is that the compiler shouldn't consider code to be concurrent until you explicitly opt into using concurrency (e.g. using annotations like @Sendable). By default, code is synchronous, and closures that are formed cannot be run concurrently. The problem in this GitHub issue is purely an incremental migration problem. The Swift 6 language mode will prevent you from passing a closure over an isolation boundary unless that closure is @Sendable. To catch actor isolation violations in code you don't own and that the compiler cannot see (i.e. because the implementation is across a module boundary), that's when the dynamic actor isolation checking from SE-0423 kicks in.

I also filed these two enhancements for the dynamic actor isolation checking to provide more actionable guidance when you hit one of these assertions, and to eliminate false-positives when the closure does not touch any actor-isolated state or methods in its body:

1. https://github.com/swiftlang/swift/issues/75508
2. https://github.com/swiftlang/swift/issues/75856

[NachoSoto]

Any updates here? We'd love to start shipping our app with Xcode 16 as soon as possible, but this bug makes it impossible for us to ship with confidence, as we know we'd be releasing dozens of unknown crashes.

[hborla]

No, I do not have any updates on this issue. As always, I will post here when I have an update to share.

We'd love to start shipping our app with Xcode 16 as soon as possible, but this bug makes it impossible for us to ship with confidence, as we know we'd be releasing dozens of unknown crashes.

I assume you specifically mean with Swift 6 language mode adoption, right? Note that this issue does not impact projects built with the Swift 6.0 compiler in the Swift 5 language mode, even if you are building with -strict-concurrency=complete. This issue only impacts projects built with the DynamicActorIsolation upcoming feature or the -enable-actor-data-race-checks debugging flag enabled, and neither of these flags are automatically enabled under complete concurrency checking.