withUnsafeContinuation can break actor isolation

[groue]

Describe the bug

Hello, withUnsafeContinuation is able to break actor invariants that should be protected by actor isolation.

Steps To Reproduce

The following test instantiates one actor, and performs as many increments as decrements of an isolated count property. It tests that the final value is the initial one: zero.

func test_that_UnsafeContinuation_can_not_break_actor_isolation() async {
    actor MyActor {
        var count = 0
        
        func zero() async {
            count += 1
            await withUnsafeContinuation { continuation in
                count -= 1
                DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
                    continuation.resume()
                }
            }
        }
    }
    
    do {
        let actor = MyActor()
        await withTaskGroup(of: Void.self) { group in
            for _ in 0..<1_000 {
                group.addTask {
                    await actor.zero()
                }
            }
        }
        
        let count = await actor.count
        // Failure varies:
        // XCTAssertEqual failed: ("1") is not equal to ("0")
        // XCTAssertEqual failed: ("4") is not equal to ("0")
        // XCTAssertEqual failed: ("7") is not equal to ("0")
        XCTAssertEqual(count, 0)
    }
}

Expected behavior

The above test passes without failure.

Environment (please fill out the following information)

• OS: macOS 12.5.1 (21G83)
• Xcode Version/Tag/Branch: Version 14.0.1 (14A400)

Additional context

I stumbled upon this buggy (right?) behavior while exploring the difficulties using unsafe continuations in this discussion about the implementation of a counting semaphore for swift concurrency: groue/Semaphore#2 (reply in thread).

[groue]

Adding a lock solves the issue:

func test_that_UnsafeContinuation_can_not_break_actor_isolation() async {
    actor MyActor {
        let lock = NSLock()
        var count = 0
        
        func zero() async {
            lock.lock()
            count += 1
            await withUnsafeContinuation { continuation in
                count -= 1
                lock.unlock()
                DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
                    continuation.resume()
                }
            }
        }
    }
    
    do {
        let actor = MyActor()
        await withTaskGroup(of: Void.self) { group in
            for _ in 0..<1_000 {
                group.addTask {
                    await actor.zero()
                }
            }
        }
        
        let count = await actor.count
        // Always good
        XCTAssertEqual(count, 0)
    }
}

Not only this lock is not supposed to be necessary, but the compiler emits warnings, probably due to SE_0340 Unavailable From Async Attribute:

Instance method 'lock' is unavailable from asynchronous contexts; Use async-safe scoped locking instead; this is an error in Swift 6

But this annoying warning is not what is interesting. What is interesting is that the lock workaround reveals that the source of the issue is probably the task resumed by the withUnsafeContinuation suspension point (not the current task, but the eventual task that is resumed from the suspension point, before the withUnsafeContinuation closure is executed).

It is impossible to reason about the behavior of this resumed task, and its interactions with the current task. One has to assume the possibility of many kinds of bad interactions. According to this issue, it looks like the authors of the Swift actor runtime did not foresee how bad these interactions can be.

[LucianoPAlmeida]

cc @ktoso @kavon

[kavon]

The withUnsafeContinuation is suppose to inherit the executor of its caller, so despite there being an await, that's not a place where it's suppose to give up the actor. That's also why the type-checker allows you to mutate the count inside the block receiving that continuation. So the +1 and -1 should happen all in one go.

I believe you're seeing this behavior due to an unfortunate mismatch observed by others between the stdlib and compiler being when using Xcode 14.0.1 on macOS 12.

In the Swift 5.6-era, there was no @_inheritsExecutor attribute because the compiler enforced that by default. For withUnsafeContinuation, which is defined in Swift's stdlib, inheriting the executor is also an important part of its semantics. But with Swift 5.7 the compiler does require that annotation, or else it will not inherit the executor. Thus, the mixing of the old Swift 5.6 stdlib (which is part of the macOS 12 SDK) with the newer Swift 5.7 compiler by Xcode 14.0.1 will cause this bug. Sorry you ran into this.

This shouldn't reproduce if you use Xcode 13 on macOS 12. If it does, then something else is going on.

[groue]

Thank you @kavon. This is an "unfortunate mismatch" indeed. I guess this issue is moot, since there is nothing to do, except avoiding Xcode 14.0 and 14.0.1 (maybe future versions as well, I don't know) to build macOS targets. Who knows how many people are doing just that just right now? 🤷‍♂️

[kavon]

The Xcode 14.1 betas do not have this problem and is available for download.

[groue]

It's difficult to grab a clear answer to this question, so... may I ask here? Betas will turn into RC and stable release eventually. And one can't submit an app to the App Store with a beta. Will we forever be stuck with Xcode 13.2 and Swift 5.6 if we target macOS before macOS 13 Ventura?

[kavon]

Will we forever be stuck with Xcode 13.2 and Swift 5.6 if we target macOS before macOS 13 Ventura?

No. The behavior you reported is nothing more than a bug specific to Xcode 14.0.x.

[groue]

Thank you. I can't tell you how many hours I've lost on this, and how many hypothesis I've built (garbage in, garbage out, right?). Xcode 14.0.x go to the Trash!