[Returned for revision] SE-0472: Starting tasks synchronously from caller context

I'll respond to @ktoso's comment from the review here, just to maintain a linear conversation.

Okay. I think you probably still want something like @_inheritActorContext here, because it's important for usability that that function pick up on the current isolation when you know it. If you don't, then closures passed in here are going to default to being nonisolated, and programmers will have to explicitly make them isolated in order to do things that they'll expect to be able to do with the current isolation. (This is basically the same type signature and scenario that happens with task groups.) I think it's going to be very common to use this from an isolated context, and it would be unwelcome and surprising for the new task to start nonisolated.

You're absolutely right that you can't rely on the function being isolated the same as the current context, but that's not why I'm suggesting it.

Agreed, and I think it's fair for us to hold off on doing the warning work to see if it's a real problem in practice.

Hmm, this is interesting; let's think it through.

First off, I don't think the runtime should be playing much of a role here. The task function is an async function, and async functions are generally assumed to handle their own isolation. The caller doesn't need to (and generally should not) eagerly switch to the callee's isolation before making the call. It looks like you've got the runtime proactively deciding whether to run the initial task funclet synchronously or enqueue it, but I don't think that's really useful: regardless of what the runtime does, the first thing that that funclet's going to do is turn around and ask the runtime to switch to the right executor. The result is that you might as well just run the funclet synchronously, and it'll presumably make its executor request, and if that needs to suspend, it'll suspend and enqueue the task. The result is that you're essentially just forcing the executor check to be done twice. (You do need to run the initial funclet in a special runtime context that disallows switching executors on the current thread, though.)

Now, it's an interesting question what the function will try to do. If we infer it to be nonisolated, under SE-0461 that should mean it preserves its caller's isolation, right? And maybe under that logic it shouldn't be trying to switch to the generic executor and thus immediately suspending because the current context is isolated. However, I don't think that's actually how nonisolated currently interacts with @isolated(any); I believe a nonisolated function that's converted to @isolated(any) effectively becomes @concurrent. So it will switch to the generic executor.

Yes, I agree that capturing the isolation would be cleaner even for Task.init. However, @hborla has some very reasonable concerns that this could lead to new reference cycles, which means we can't rely on a change there being viable, and even if it happens, it will need real investigation first.

Assuming that I'm correctly analyzing the isolation and scheduling rules above, I think we have a more urgent need for this API to preserve isolation than for Task.init. The problem is that we can end up reliably violating the explicitly-expressed intent of this API in the default, unannotated case:

actor A {
  func foo() {
    print("Hello, ")
    Task.startSynchronously {
      // doesn't capture self
      print("world!")
    }
  }
}

foo is statically known to be isolated. If the task function does not inherit that isolation and therefore (by the argument above) becomes effectively @concurrent, it will start by trying to switch to the generic executor, and so the entire function will run asynchronously. I think that's a major problem.

2 Likes