Cannot understand the nature of data race warnings in Swift 5.10

With update to Swift 5.10 I have got many warnings, all saying the same:

warning: passing argument of non-sendable type 'B' outside of actor-isolated context may introduce data races

I took out a few of them out of Apple SDKs to understand it better without any luck. So will appreciate any help on why there is a warning, and maybe where to read more on why it is a warning here? And how to correctly resolve it.

First example is simplified part of SwiftUI view, where a task group is created.

@MainActor
struct Wrapper {
    func test() async {
        await withTaskGroup(of: Void.self) { group in
            for _ in 0..<100 {
                group.addTask {
                    print("Hello, world!")
                }
            }
            await group.waitForAll() // warning: passing argument of non-sendable type 'inout TaskGroup<Void>' outside of main actor-isolated context may introduce data races
        }
    }
}

I have managed to go await this warning by making this method nonisolated, yet I do not understand why?

The second example is an actor that uses other non-sendable class:

actor A {
    private let b = B()

    func prepare() async {
        await b.prepare() // warning: passing argument of non-sendable type 'B' outside of actor-isolated context may introduce data races
    }
}

final class B {
    func prepare() async {
    }
}

If I pass isolated A as parameter to B, the issue is gone since now prepare is isolated on an actor, but is that correct way? Or something should be done completely differently here?

Finally, I have top-level execution code where async calls being wrapped in a task, and it also gives data race warning, while I assumed it should be OK since everything is happening in main actor isolation context.

@MainActor
protocol Commands {
    func load()
}

final class List {
    func load() async {
    }
}

@MainActor
struct ErrorHandler {
    func task(_ execute: @escaping () async throws -> Void) {
        Task {
            try! await execute()
        }
    }
}

struct CommandsImpl: Commands {
    let list = List()
    let handler = ErrorHandler()

    func load() {
        handler.task {
            await list.load() // warning: passing argument of non-sendable type 'List' outside of main actor-isolated context may introduce data races
        }
    }
}
2 Likes

At least the second and third, if I understand correctly, have to do with switching from actor-isolation to nonisolated involving a non-sendable type.

I have just bee thinking about this (coming in from a different question) here:

In all of your code examples, there is an instance of a non-Sendable type being passed outside of an actor-isolated context when calling an async function that is not isolated to any actor. When you have a function that is async and nonisolated, it is always evaluated on the global concurrent thread pool, which is also called the "generic executor". The generic executor manages the global concurrent thread pool. This behavior of nonisolated async functions is detailed in SE-0338: Clarify the Execution of Non-Actor-Isolated Async Functions. So, this means that while your function on your actor (or the main actor) is suspended waiting for the nonisolated async function to return, the calling actor is freed up to run other work, which could end up accessing the non-Sendable state that was passed off to the function, leading to concurrent access of non-Sendable state! That's the reason for the warnings.

It's because withTaskGroup accepts a non-Sendable closure, which means the closure has to be isolated to whatever context it was formed in. If your test() function is nonisolated, it means the closure is nonisolated, so calling group.waitForAll() doesn't cross an isolation boundary.

I think your workaround is the right one for now, but this specific waitForAll() API should be fixed in the Concurrency library itself.

Yes, using an isolated parameter to prepare() will resolve the issue because prepare() will run on the actor. Whether or not that's the correct thing to do depends on what you're trying to do in prepare(), but if your intention is to always isolate access to B in that actor, adding the isolated parameter is the right way to accomplish that.

If you'd like everything in this code to happen on the main actor, you can mark List as @MainActor to resolve the warning:

@MainActor
protocol Commands {
  func load()
}

@MainActor
final class List {
  func load() async {
  }
}

@MainActor
struct ErrorHandler {
  func task(_ execute: @escaping () async throws -> Void) {
    Task {
      try! await execute()
    }
  }
}

struct CommandsImpl: Commands {
  let list = List()
  let handler = ErrorHandler()
  
  func load() {
    handler.task {
      await list.load()
    }
  }
}

Note that adding global actor isolation like @MainActor to a class makes that class Sendable. You can pass around an instance of the class all you want, but to access its mutable state, you need to get back on the main actor!

6 Likes

Seems like I've been missing on important piece and understood async functions behaviour incorrectly. Thanks, this proposal has clarified some details for me!

So in the way I am using it in this particular case, this isolation of B on A is what I need. And if I do not want to make it isolated on the actor, I need it to be sendable, since it is possible to create two tasks that call A.prepare and therefore access B which is not safe to access in that way, is that correct?

With that so far I have the least understanding on an options to approach... I would prefer List to be not isolated on a certain actor by design, but rather specify this isolation later. So far it has been possible to achieve this by this implementation I provided, not in obvious way, but it worked. Now I understand why it is not working that way anymore after reading the proposal, yet have no idea how to change it without binding List to the (main or any other global) actor?

1 Like

To be precise, I can specify an actor isolation parameter here as well, but this will require:

  1. Make all such methods on the List class(es) to accept actor as parameter.
  2. Until Swift 6, ensure that all calls receive correct actor when being called.

The second point is reasonable, since concurrency is not complete so far. But for the first I do not see a solution (maybe I am missing something, of course).

Being more explicit on execution context is a good thing, so I would like to specify it explicitly per use case, like...

struct CommandsImpl: Commands {
    @isolated(MainActor.shared)
    private let list = List()
    
    func load() {
        handler.task {
            await list.load() // now this will be isolated on main actor
        }
    }
}

...to make all the calls to the list isolated on the main actor. This attribute on a property, probably, has little sense matching to the effect, but so far I have no better expression to that.

2 Likes

Yes, that's right. the calls to A().prepare() on the same actor value can't be run in parallel, but the code in your second call to A().prepare() can run concurrently with the first call to b.prepare() if b is not isolated to the actor that stores it. The behavior of actors where they are freed up to do other work while waiting on an async function to finish is called actor re-entrancy, and it's what allows Swift concurrency to always make forward progress.

Right, isolated parameters are the only way to accomplish the effect you're describing right now. There are some other ideas for isolating an entire non-Sendable type to an actor value being discussed over in Isolation Assumptions - #56 by Nickolas_Pohilets

1 Like

I felt that this thread is related :slight_smile: Big thanks on clarification on all the questions!

1 Like

Hi, I think I must be misunderstanding something, but SE 0313 says:

The types involved in a non-isolated declaration must all be Sendable, because a non-isolated declaration can be used from any actor or concurrently-executing code.

So, IIUC parameters and return value of a nonisolated method or closure must be of Sendable types. Since withTaskGroup()'s closure takes a parameter of TaskGroup type, which is non-Sendable, why is it valid to run withTaskGroup() in a nonisolated function? What am I misunderstanding? Thanks for any explanation.


EDIT: Sorry, I probably figured out why. The issue isn't with withTaskGroup()'s closure, which runs in the same context as its caller, but with the await group.waitForAll(), because async func runs in global pool. Changing test() to run in global pool too avoids concurrency domain change. That's why it works.

As for why it's OK to run withTaskGroup() in global pool, my assumption that only nonisolated methods could run in global pool seems incorrect. Below are my new understanding about calling an async func (suppose it's not an actor's method and doesn't have global actor attribute applied):

  • If the caller is actor-isolated, only nonisolated async func can be called.

  • If the caller is non-isolated, any async func can be called, because the call doesn't cross concurrency domain.

From my understanding of the withTaskGroup in general should not be bothered from where it is called at all, it simply should inherit isolation (which it does not for some of the methods due to the lack of mechanisms earlier in the language + pre-SE-0338 behavior at the time it was first introduced).

I agree what you said is correct, because, for example, it's consistent with the output of the example code in this post.

However, do you know where is this behavior documented? I can't find it in SE 304 (structured concurrency). SE 304 says "By default, the task group will schedule child tasks added to the group on the default global concurrent executor". But that's about child tasks not the code in withTaskGroup()'s closure. The slide in this post doesn't help for the same reason.

My hypothesis is that when SE 304 was implemented, the default behavior to run an async func was to run it on the current executor (see the old behavior described in SE 0338). So the behavior of withTaskGroup() wasn't different from other async funcs at that time. That's why SE 304 didn't describe it explicitly. However, after SE 0338 was implemented, the behavior of withTaskGroup() became an exception but isn't documented anywhere. Does anyone know if this understanding is correct? Shouldn't this behavior be documented explicitly?

1 Like

The only "documentation" I know of is the presence of the @_unsafeInheritExecutor attribute in the standard library source code for withTaskGroup. So withTaskGroup uses the same mechanism as e.g. withCheckedContinuation to opt out of the SE-0338 semantics.

Unfortunately, the official documentation hides underscored attributes/protocols/etc., even if they're important for understanding how things work. I wish that weren't the case.

I understand there's ongoing work to replace @_unsafeInheritExecutor with the new #isolation feature introduced in SE-0420, which would make the behavior apparent from the function signature.

@ktoso recently landed this change for the with…Continuation APIs: [Concurrency] Remove _unsafeInheritExecutor from public APIs, use #isolation by ktoso · Pull Request #72578 · apple/swift · GitHub. I assume the same is planned for the other stdlib APIs that use the attribute.

3 Likes