Soundness hole in isolated protocol conformances

Using Swift
1

From my reading of SE-470, the following code shows a soundness hole in isolated protocol conformances:

protocol Foo {
  func bar()
}

@MainActor struct S: @MainActor Foo {
  var count = 0
  func bar() {
    if !Thread.isMainThread {
      print(count, Thread.current)
      MainActor.assertIsolated()
    }
  }
}

@Test func test() {
  let s: Any = S()
  // This cast should fail since 's' is '@MainActor Foo'
  if let s = s as? any Foo {
    func open(_ foo: some Foo) { foo.bar() }
    open(s)
  } else {
    // Instead we should get into here, but we don't
  }
}

Here we are able to erase a @MainActor Foo type to Any, and then dynamically cast it as any Foo. This allows us to run bar on a background thread. We are even able to access count on a background thread without crashing, which seems like another soundness hole. The only way I could get this to crash is to perform an explicit MainActor.assumeIsolated(). This code compiles without warnings/errors in Swift 6 mode.

Here is the passage from SE-470 that makes me think this should not be allowed:

One last issue concerns dynamic casting. Generic code can query a conformance at runtime with a dynamic cast like this:

nonisolated func f(_ value: Any) {
  if let p = value as? any P {
    p.f()
  }
}

If the provided value is an instance of C , and this code is invoked off the main actor, allowing it to enter the if branch would introduce a data race. Therefore, dynamic casting will have to determine when the conformance it depends on is isolated to an actor and check whether the code is running on the executor for that actor.

I read this as saying the compiler "will have to determine", not the programmer. But maybe I'm wrong!

4 Likes
2

Smaller version of test function gets to crash it either for me:

func test() {
    let s = S()
    func open(_ foo: some Foo) {
        foo.bar()
    }
    open(s)
}

results in Fatal error: Incorrect actor executor assumption; Expected 'UnownedSerialExecutor(executor: (Opaque Value))' executor.

3

I think you're right that as? is missing the check required by the SE, but I also think there's another few holes in the SE:

Protocols that extend Sendable shouldn't be eligible for isolated conformances, else:

import Dispatch

protocol P: Sendable {
    func f()
}

@MainActor
struct S: @MainActor P {
    func f() {
        MainActor.preconditionIsolated()
    }
}

@MainActor
func example(_ s: any Sendable) {
    let p = s as! any P // on the main actor, so the cast is allowed dynamically
    DispatchQueue.global().async {
        p.f() // oops, sent it anyway
    }
}

example(S())
dispatchMain()

But even without Sendable, you can still get into trouble:

import Dispatch

protocol P {
    func f()
}

@MainActor
struct S: @MainActor P {
    func f() {
        MainActor.preconditionIsolated()
    }
}

@MainActor
func example(_ s: sending Any) {
    let p = s as! any P // cast is dynamically OK
    Task.detached {
        p.f() // oops, sending
    }
}

example(S())
dispatchMain()

For the second example, I don't see a solution except to have casts join the result to the region of the casting code?

(@Douglas_Gregor)

3 Likes
4

I went ahead and created an issue for this soundness hole: Soundness hole in isolated conformances · Issue #82539 · swiftlang/swift · GitHub

2 Likes
5

I've created an issue for the two knock-ons I found in this thread: Two soundness holes casting isolated conformances (SE-0470) · Issue #82550 · swiftlang/swift · GitHub

1 Like
6

Are you on an ABI-stable platform with an older runtime? If so, this is a known—and unpluggable—hole:

2 Likes
7

Ah, thank you for pointing out this extra info. Testing on a newer runtime does show the correct behavior.

And it's nice that this soundness hole is pointed out in the proposal under "ABI compatibility", but it would be even nicer to have a kind of "Important:" callout directly in the proposal where the stated information differs on older runtimes. Even something as simple as this.

3 Likes