Should global-actor-isolated functions be implicitly Sendable?

I was surprised today to find that the following code produces a warning:

@MainActor class C {
    var x: Int = 0
    func inc() {
        x += 1
    }
}

@MainActor
func g() async {
    @MainActor
    func f() async {} // warning: concurrently-executed local function 'f()' must be marked as '@Sendable'; this is an error in Swift 6

    let c: C = C()

    Task.detached {
        await f()
        await c.inc()
    }
}

The warning is fixed by marking f as @Sendable, but should we have to do that? The call to c.inc() succeeds fine due to this rule from SE-0316:

A non-protocol type that is annotated with a global actor implicitly conforms to Sendable .

I don't recall whether this came up in the midst of the concurrency proposals, but is there any reason this courtesy isn't extended to functions?

7 Likes

I now have run into something that I think is similar/same, and I'm really struggling to understand.

@MainActor
class MyClass {
	func doThing() {
	}
}

func test() {
	let closure: @MainActor () -> Void = {
		print("hmmmm")
	}

	let value = MyClass()

	Task {
		// warning about capturing non-Sendable
		await closure()

		// this is fine
		await value.doThing()
	}
}

Under what conditions would this actually be unsafe?

1 Like

I think this is an excellent question, and I think the answer is yes, isolated functions should implicitly be @Sendable. If you form a global actor isolation closure from off the actor and capture non-Sendable state, that closure is not callable today. And actor isolation implying Sendable-ness matches everybody's intuition because it's exactly what we tell folks about actor isolated types.

The only question is how this interacts with region isolation SE-0414. If you form a non-Sendable isolated closure off the actor and capture non-Sendable state, that closure still is callable if the captures are in a disconnected region that can be transferred. I think that's still okay; we could make isolated closures be @Sendable by default unless you have written some other annotation to indicate that the closure is not-Sendable but it may be transferred (e.g. with a disconnected annotation per the future directions of SE-0414).

4 Likes

That's great news!

I also opened a bug about this: Could it be closures with global actors should be Sendable? · Issue #71098 · apple/swift · GitHub

2 Likes

FWIW there's at least some value in having a non-Sendable global-actor-isolated closure, though it seems pretty niche. Here's a situation in which your original example can be unsafe:

func test() {
    var x = 0

    let closure: @MainActor () -> Void = {
        x += 1 // data race!
    }

    Task {
        // (valid) warning about capturing non-Sendable
        await closure()
    }

    while true { x += 1 }
}

Specifically, a non-Sendable global-actor-isolated closure can access shared mutable state and capture non-Sendable values. Annotating the above closure with @Sendable would cause it to not compile because of the shared mutable capture of x across isolation domains.

Hmm maybe I'm missing something but it seems like I can indeed call this? (5.10 with Strict Conurrency enabled.) Though it's a pretty contrived example that's unlikely to actually be useful:

func captureDeviceState() async {
    var deviceState: [String: String] = [:]
    deviceState["wifiSpeed"] = await doSpeedTest()
    await { @MainActor in
        // UIDevice is @MainActor
        deviceState["version"] = UIDevice.current.systemVersion
        deviceState["model"] = UIDevice.current.model
    }()
}

Funny enough, I first tried to do this with MainActor.run, assuming that it would take a non-Sendable global-actor-isolated closure but surprisingly it appears to spuriously require that its closure be Sendable. Appears legal to declare something like:

extension MainActor {
  public static func runNonSendable<T: Sendable>(
    _ closure: @MainActor /* not @Sendable */ () throws -> T
  ) async rethrows -> T {
    try await closure()
  }
}

but I do think it makes sense to have a global actor annotation imply @Sendable by default. IIUC adding a global actor to a closure is isomorphic to something like:

class Closure {
  var xCapture: Int = 0
  @MainActor func invoke() { ... }
}

which is why Closure isn't Sendable. Meanwhile one might expect it to instead be

@MainActor class Closure {
  var xCapture: Int = 0
  func invoke() { ... }
}
2 Likes

I agree this is unsafe, but is it possible this is a hole? Help me understand why that should be allowed when this definitely is not:

var x = 0 // Var 'x' is not concurrency-safe

@MainActor
func test2() {
    x += 1 // Reference to var 'x' is not concurrency-safe
}

hmm which part specifically? I think the only way to trigger a data race here would be to send closure across isolation domains and invoke it, which the compiler correctly diagnoses with a warning here — so I don't think you can make this an unsound program without encountering warnings (presumably errors in Swift 6).

At first I also thought that this was a hole:

func test() {
  var x = 0
  
  let closure: @MainActor () -> Void = {
      x += 1
  }

  // increments x on the main thread. racy??
  await closure()
}

but then I realized that the non-Sendability of closure means that it must be invoked within the existing Isolation Region, so you can't run it in "parallel" with any other code that mutates x. And although the bona fide Isolation Regions proposal hasn't landed yet, it appears that closures have supported this concept for a while.

Or at least that's the way I understand it — maybe Holly or someone else can shed more light on the topic. This has definitely tested my assumptions about Swift Concurrency too; your Mastodon post from yesterday may have nerd-sniped me for several hours as I attempted to wrap my head around this :laughing:

2 Likes

What I'm getting at is here:

// this function has no isolation
func test() {
    var x = 0

    let closure: @MainActor () -> Void = {
        // why are we allowed to capture mutable x here?
        x += 1
    }

I was just playing around with this, and I think you may be right. I cannot come up with any example so far that seems both incorrect and also does not produce a diagnostic.

I can! Check this out:

@globalActor
actor MyActor {
  static let shared = MyActor()
  @MyActor static var ns: NotSendable?
  @MyActor static func ohNo() { ns!.x += 1 }
}

@globalActor
actor YourActor {
  static let shared = YourActor()
  @YourActor static var ns: NotSendable?
  @YourActor static func ohNo() { ns!.x += 1 }
}

class NotSendable {
  var x: Int = 0
}

@MyActor func exhibitRace() async {
  let ns = NotSendable()
  MyActor.ns = ns

  // prepare the race
  await { @YourActor in
    YourActor.ns = ns // <--- HERE:
    // non-Sendable state captured in a non-Sendable closure
    // isolated to something else and passed across an isolation
    // boundary when the closure is called
  }()

  // And now we can access the 'NonSendable()' value above
  // in parallel!
  await withTaskGroup(of: Void.self) {
    $0.addTask {
      await MyActor.ohNo()
    }

    $0.addTask {
      await YourActor.ohNo()
    }
  }
}

await exhibitRace()

I can totally understand why nobody has run into this. Because all of the task APIs take @Sendable closures, this is pretty difficult to accidentally write because the code has to be written in this really specific way to avoid capturing the non-Sendable value in a @Sendable closure. Basically, the non-Sendable isolated closure has to be both formed and called from some other isolation domain where the non-Sendable value is created, which I expect is not a very common thing to do. When people want to form a closure in another isolation domain that is then called, they typically use Task { ... } which has proper Sendable enforcement. And then on top of that, you have to have saved the non-Sendable value in isolated state in both isolation domains so that it can later be accessed concurrently.

3 Likes

Wow. I'm interpreting this to mean that:

  • this is in fact a hole in Sendable checking
  • fixing this hole is compatible with making globally-isolated function types Sendable

:sweat_smile:

Is that right? If so, the hole seems like it's just a bug.

Will making globally-isolated function types Sendable require an evolution proposal?

Indeed. Fortunately, the Sendable checking hole is already filled on main under -enable-experimental-feature RegionBasedIsolation, because capturing a non-Sendable value in an actor isolated closure will transfer the value into the actor's region, meaning it's unsafe to access the non-Sendable value in the original isolation domain.

I think so; I always err on the side of writing proposals, even if they're relatively minor, because the proposal documents the semantic change, and you never know who might come up with some reason why the proposal should be revised in some way!

4 Likes

I just tested a recent main snapshot. Your snippet indeed produced a warning when compiled with RegionBasedIsolation, but I was able to modify it to build without a warning in a way that still appears to be racy/unsound:

@globalActor
actor MyActor {
  static let shared = MyActor()
  @MyActor static var ns: NotSendable?
  @MyActor static func ohNo() { ns!.x += 1 }
}

@globalActor
actor YourActor {
  static let shared = YourActor()
  @YourActor static var ns: NotSendable?
  @YourActor static func ohNo() { ns!.x += 1 }
}

class NotSendable {
  var x: Int = 0
  
  @MyActor init() {
    MyActor.ns = self
  }
}

@MyActor func exhibitRace() async {
  let ns = NotSendable()
  
  await { @YourActor in
    YourActor.ns = ns
  }()
  
  await withTaskGroup(of: Void.self) {
    $0.addTask {
      await MyActor.ohNo()
    }
    
    $0.addTask {
      await YourActor.ohNo()
    }
  }
}

Am I missing something here? I'm using the toolchain from this build, with RegionBasedIsolation and StrictConcurrency.

2 Likes

This is safe to do, because ns is not used again from @MyActor after transferring it into @YourActor within the closure, so it's impossible to access that non-Sendable value concurrently. This behavior and why it's safe is detailed in the SE-0414 proposal.

EDIT: I see, the issue is that calling NotSendable.init makes the value always part of MyActor's region. This is just a bug in the region isolation implementation. I've filed Global actor isolated initializers of non-`Sendable` types should cause the value to be in the actor's region · Issue #71533 · apple/swift · GitHub

4 Likes

looks like it isn't just initializers btw: this also compiles without warnings

...

class NotSendable {
  var x: Int = 0

  @MyActor func stash() {
    MyActor.ns = self
  }
}

@MyActor func exhibitRace() async {
  let ns = NotSendable()
  ns.stash()

  await { @YourActor in
    YourActor.ns = ns
  }()

  ...
}
2 Likes

Thank you, I've noted that issue in Global actor isolated initializers of non-`Sendable` types should cause the value to be in the actor's region · Issue #71533 · apple/swift · GitHub

3 Likes

I've fixed the issue conservatively under -strict-concurrency=complete at [Concurrency] Don't allow non-`Sendable`, isolated closures to capture non-`Sendable` values that may be isolated to a different actor. by hborla · Pull Request #71536 · apple/swift · GitHub. Thanks for the test cases!

5 Likes

Whipped something up here: