`@isolated(any)` function types

Modifiers and attributes are generally adjective phrases, although there are certainly exceptions. And we've already got nonisolated and isolated as precedents here. You're right that global actor attributes are in this same space and are one of those exceptions; I feel like that's an understandable exception, and it's better to follow the general rule. I also named this attribute with the expectation that we'd have related features that'd want to use the same @isolated attribute, like @isolated(caller) and so on. I think it's better to have these features be grouped and not have a million ad hoc names. I also think we should stay away from adding more built-in attributes with capitalized names — there are only a few existing ones, and they all do framework-specific work that could certainly now just be a macro.

In a future where we allow that, we should model builtin properties like this as if they were magically declared in the Swift module. I think that's a better solution to any source-compatibility issues than forcing builtin properties to have non-standard spellings.

2 Likes

We've run into a snag here. SE-0392: Custom Actor Executors says in its description of Executor.enqueue:

Eventually, an executor will want to actually run a job. It may do so right away when it is enqueued, or on some different thread, this is entirely left up to the executor to decide.

This phrasing explicitly allows executors to execute the submitted job synchronously during enqueue. However, the Concurrency runtime does not currently support this correctly, at least not in general: the runtime doesn't correctly save and restore its internal tracking of the current task. As a result, if the current thread is running a task, and it reentrantly tries to run another task, the second task will leave the runtime with an invalid task reference when control returns to the first task. The first task will then likely crash if it tries to do any real work.

There is a case where running a task reentrantly does reliably work: if the current task is being suspended anyway. The most important situation where that can happen is when the current task is being suspended because it's trying to switch to the actor in question, and so the enqueued task is the same as the current task. I suspect that people sometimes think of SerialExecutor.enqueue as the operation a task does to switch actors, and so they may be tempted to use synchronous execution during enqueue as a way to quickly switch to the actor without suspending the task. This will currently fall into this special case and work.

However, there are other times that a task needs to be enqueued:

  • when it's just been freshly created
  • when it's currently waiting on a continuation that's just been resumed
  • when it's currently waiting on a task that just completed
  • when it's currently waiting on a task group, and one of the group tasks just completed

It'd be bad for enqueue to synchronously block during any of these, even if this bug didn't corrupt the current task; code may reasonably be written to expect that enqueue finishes quickly and does not misbehave if e.g. locks are held during the call. And enqueue doesn't get told which case it's in and why the task is being enqueued. So while I'm sympathetic to people trying to optimize custom executors in this way, I think it's the wrong way to do it.

Notably, however, the Swift compiler currently does not directly enqueue tasks on an actor in any of these situations. The compiler doesn't give the runtime any specific information about where to run, so the runtime just enqueues on the generic concurrent executor. As a result, people doing synchronous execution during enqueue have probably been getting away with it without consequences even for these cases:

  1. The task gets directly enqueued on the generic concurrent executor instead of the actor's executor.
  2. The generic concurrent executor presumably starts running the task without synchronously blocking any other tasks.
  3. The task tries to enqueues itself on the actor, which runs it synchronously, falling into the special case above where reentrant task execution works.

@isolated(any)'s ability to directly enqueue on the formal isolation of the task function will be the first thing to exceed those limitations. That also means it will directly expose this bug with reentrant task execution for the first time[1]. You will see this if a task creates another task that starts isolated to an actor whose executor uses synchronous execution during enqueue.

We can fix the runtime so that running tasks reentrantly this way doesn't leave the runtime in an inconsistent state. We should do that anyway, and I don't think the overhead of that is likely to be significant. I'm not sure at this time whether we can fix it for existing runtimes on our platforms with stable ABI. Even if we can't, currently-compiled code on those platforms should still continue to work because of those limitations of current compilers re: directly enqueuing tasks on actors. However, if you recompile code with a future compiler including @isolated(any), and your code includes an executor that does synchronous execution during enqueue, you need to be aware of two things:

  1. Previous OS releases may have this bug with reentrant execution of tasks, and that bug may be exposed when creating tasks on your actor that does synchronous execution during enqueue.
  2. enqueue will be called in more contexts, including some that you probably should not block.

  1. Sort of. You could have seen this bug yourself if you implemented an executor using tasks. ↩︎

7 Likes

Thanks for calling this out. I was under the impression that the enqueue methods are not allowed to synchronously run the job. I personally ran into these memory corruptions myself when adopting both serial and task executors in NIO; hence, NIO is always enqueuing the job into its queue and runs it earliest with the next loop iteration.

Do you know if we have any serial or task executors that directly run an enqueued job somewhere?

I only know of them in test code.

Thanks for the in depth writeup @John_McCall -- I don't think this wording and ability was specifically requested/designed and is more of a slip up in wording really.

Our own test code may have just been doing this out of lazyness in order to not pull any real execution into some of the tests, but I'm certain the tests don't "require" this. I have also not heard of anyone specifically using this -- especially as it would be half broken in practice as @FranzBusch seems to have observed in practice.

The plan here sounds right to fix this anyway, but perhaps in the meantime it'd be ok to avoid the proposal claiming such capability via amending that section?

It does not seem like API docs on enqueue claim such capability either (tbh because enqueue is pretty much undocumented), so the only place worth amending (with either a warning or just change) would be the proposal text -- and we can add proper API docs to the Executor.enqueue perhaps instead?

Random side discussion: In practice I've seen "calling thread executor" in reality but only used sparingly with Futures to avoid executing a map asynchronously etc... We've done such in Akka's HTTP pipeline for example. So I see the general utility of it; but in our Swift Concurrency world the same is achieved with "stay on the same task executor" so perhaps such calling thread executor won't be necessary even if it could work correctly.

1 Like

Being able to synchronously run a job would be really good and was the first thing I tried since this is what we do in NIO for futures. A lot of NIO code checks if we are already in the event loop and then synchronously executes else it enqueues a „NIO job“.

When trying out task executors in a NIO based HTTP server I saw performance improvements but there was still a difference because our NIO async bridges use continuations. With task executors the task that suspended on the continuation was executed on the same executor as the executor that resumed the continuation. Since we weren’t able to run synchronously in that case we always had one unnecessary event loop tick. I expect synchronously running the job would bring an additional performance benefit.

2 Likes

I think we should probably add the ability to donate the current thread when resuming a continuation.

3 Likes

Perhaps more importantly, it means that the order in which tasks are actually enqueued on the actor is not necessarily the same as the order in which they were created. It would be much better --- both semantically and for performance --- if the initializer could immediately enqueue the task on the right executor to begin with.

Finally, every task-creation API in the standard library will be updated to take a @isolated(any) function value and synchronously enqueue the new task on the appropriate executor.

If I'm understanding this correctly, this seems like it could be a really meaningful and exciting change.

Today in cases where you want or need a well-defined first-in-first-out task scheduling guarantee, I believe you have to use some sort of custom scheduler. One example I'm familiar with is the swift-async-queue library.

The documentation for that library shares this example to show how the current task scheduling behavior does not preserve ordering:

@MainActor
func testMainActorTaskOrdering() async {
    actor Counter {
        func incrementAndAssertCountEquals(_ expectedCount: Int) {
            count += 1
            let incrementedCount = count
            XCTAssertEqual(incrementedCount, expectedCount) // often fails
        }

        private var count = 0
    }

    let counter = Counter()
    var tasks = [Task<Void, Never>]()
    for iteration in 1...100 {
        tasks.append(Task {
            await counter.incrementAndAssertCountEquals(iteration)
        })
    }
    // Wait for all enqueued tasks to finish.
    for task in tasks {
        _ = await task.value
    }
}

Will the changes in this proposal update the behavior such that the tasks in that example are guaranteed to run in the order they're created?

I can't quite tell based on my read of the proposal / this thread so far. I think that, in order for the task order to be guaranteed, the tasks would need to be synchronously enqueued on the Counter actor. Is that right?

The task is created in a @MainActor function but just immediately enqueues work on the Counter actor. Is the task inferred to be @MainActor isolated or isolated to the Counter actor?

// Is this task isolated to the Main Actor or to the Counter actor?
Task {
  await counter.incrementAndAssertCountEquals(iteration)
}

We can also ignore the question about inferred isolation and instead explicitly isolate the task to the Counter actor:

// Since this is explicitly isolated to the Counter actor, are the
// tasks enqueued synchronously, guaranteeing a FIFO execution order?
// (is this the right way to explicitly isolate a task to a non-global actor?)
Task { [isolation counter] in
  // (and, since we're already isolated to the counter, this doesn't need to await?)
  counter.incrementAndAssertCountEquals(iteration)
}

Since the task is explicitly isolated to the Counter actor, do the changes in this proposal make it so that:

  1. the tasks are enqueued synchronously on the Counter actor?
  2. the tasks are guaranteed to run in first-in-first-out order?

And lastly, what would the behavior be if the task was instead explicitly isolated to the main actor?

Task { @MainActor in
  await counter.incrementAndAssertCountEquals(iteration)
}

Does this proposal have any effect on the behavior in that case?

1 Like

Some comments on the syntax in this example:

func delay(operation: @isolated(any) () -> ()) {
  let isolation = operation.isolation
  Task { [isolated isolation] in
    print("waking")
    operation() // <-- does not cross an isolation barrier and so is synchronous
    print("finished")
  }
}

It's really great that this will be possible!

Could it make sense to also use the @isolated annotation to support applying an explicit isolation to a closure? That could let us simplify this example to:

func delay(operation: @isolated(any) () -> ()) {
  Task { @isolated(operation) in  // seems very elegant!
    // ...
  }
}

I find this spelling really appealing because it's more similar to the syntax for isolating a closure to a global actor (both would use an @ annotation: @MainActor in and @isolated(myActor) in).

e.g. in this example from earlier, I'd find @isolated(counter) to be more intuitive and appealing:

vs

Task { @isolated(counter) in
  counter.incrementAndAssertCountEquals(iteration)
}
3 Likes

This syntax is specified in a separate proposal, which has been pitched over in

I think your feedback would be valuable to post in that pitch thread!

1 Like

Thank you! I posted similar feedback over there as well: Closure isolation control - #33 by cal

The closure being used as the task function in this example is formally isolated to the main actor. Under this proposal, the tasks will be synchronously enqueued on the main actor, which is FIFO, and then they'll switch immediately to counter, and since they all have the same priority, they'll run in their original order. Now, this proposal allows for optimization; since the closure isn't explicitly isolated to the main actor, Swift could skip enqueuing on the main actor and go direct to counter. That's actually still fine for this example unless Swift somehow does that inconsistently for different closures in the loop, which would be weird but technically possible. You could guarantee that doesn't happen by explicitly isolating the closure, in this case preferably to counter.

If the current function wasn't @MainActor, the closure would be formally non-isolated, and you'd have to explicitly isolate it (again, preferably to counter) to get an ordering guarantee. The upshot is that, if you want the ordering guarantee, you should be explicit about the isolation of your tasks.

6 Likes

This is a huge improvement! This should make it a lot easier to understand and reason about Task execution.

1 Like

Can we generalize @isolated(any) to be used with other types as well?

It looks a lot like something that can be used to box non-sendable value together with its isolating actor, thus allowing the value to be passed to other isolation domains.

class NonSendable {
    func foo()
}
actor MyActor {
    func getObject() -> NonSendable {}
}

let a = MyActor()
let obj: @isolated(any) NonSendable = await a.getObject()
await obj.foo() // hops to `obj.isolation` which is `a`
1 Like

That's an interesting idea. I think it would have to work differently from how it works for function types, though, since constructing one would always have to take the value from the current context, whereas functions have an inherent isolation that doesn't necessarily match.

I'm bringing this up, because if that’s something that we want, then we may need to reconsider that @isolated(any) implies @Sendable, because for non-functional values, boxing non-sendable values is the primary use case. And we probably want to @isolated(any) T to behave the same way for any T - functional or not.

I don't think we're likely to introduce an @isolated(any) T feature. You don't really need a language feature here, though; you could make it yourself in a library. The following should be safe:

public struct IsolatedValue<Element>: Sendable { // Note that Element can be non-Sendable
  // This value can only be safely accessed while isolated to `isolation`.
  private nonisolated(unsafe) let value: Element
  public let isolation: any Actor

  // Since this function is isolated to a specific actor, callers will assume
  // that the value will be merged into that actor's isolation region.  And,
  // in fact, it is.  Note that allowing an *optional* actor here would *not* be
  // safe because the value could then be used concurrently.
  public init(_ value: Element, isolation: isolated any Actor = #isolation) {
    self.value = value
    self.isolation = isolation
  }

  public func withValue<R: Sendable>(operation: @Sendable (Element) -> R) async -> R {
    await isolation.run {
      operation(value)
    }
  }
}

extension Actor {
  func run<R>(operation: @Sendable () -> R) -> R {
    operation()
  }
}
6 Likes