Using `async` functions from synchronous functions (and breaking all the rules)

If it is not possible to remove deprecated synchronous apis, then is that not the sign that Vapor internals can not switch to Swift concurrency yet?

It is still possible to expose public async apis by wrapping internals with continuations. Using Swift concurrency in Vapor internals would then have to wait for the next major version where synchronous apis can be removed for good.

If you want to "force" early async, take care a semaphore will deadlock if an inner async async job must run on the blocked thread. For example, blocking on the main thread would deadlock on the first @MainActor job.

Maybe it's possible to look for inspiration from RunLoop or the implementation of XCTestCase.wait(for:timeout:)?

3 Likes

Unfortunately we're stuck between a rock and a hard place on this. We could remove the deprecated APIs but then we would need another major release in the not too distant future with Swift 6 and NIO 3, which is too disruptive for the community. And we can't keep using the old internals because it's not safe - we're already seeing issues caused with async/await because many of the assumptions of Vapor are that everything is run on the same thread which isn't the case anymore, so we need to use actors etc to make that safe. We also want to use async/await as much as possible in the whole stack otherwise adopting distributed tracing is not going to be possible.

So it's entirely a problem that we could solve but it would be more disruptive in the long term for the ecosystem to do the 'right' thing now.

To be clear - you'll get deprecation warnings when you update to whatever with the new release is and if you fix the warnings there will be no use on semaphores in any of the code you touch - it's purely there to stop us breaking the API for those who can't update their code yet but we are going to strongly suggest you migrate before deploying

How about not using actors? e.g. serialising to a private queue:

class SharedData {
  private var value = 0
  private var queue = DispatchQueue(label: "SharedData_Private")

  func setValue(_ newValue: Int) {
    queue.sync { self.value = newValue }
  }

  func setValue(_ newValue: Int) async {
    await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
      queue.async {
        self.value = newValue
        continuation.resume()
      }
    }
  }
}

This pattern would ensure that synchronous calls are serialised on a new, private thread (not from the global pool), while async callers who are on pooled threads can yield those execution resources while they wait (rather than block).

It's a bit ugly - it's basically implementing your own actor, but it at least gives you the control you need to ship an async API without a new major version and without resorting to private interfaces. Eventually you could replace it with the language's provided actors.

3 Likes

As always, "custom executors" means a lot of different things to different people, so it's important to spell out what you specifically want. Here I think you're saying that, if you have a synchronous blocking function that you want to expose as async, you'd like to be able to force it to execute on a different thread pool (which may or may not be width-limited) in order to avoid creating thread-exhaustion problems in the standard width-limited global pool. Presumably that thread pool would not be acting semantically like an actor in any way, though.

1 Like

We use the class wrapper trick in swift-nio too. You just need to add @unchecked Sendable to the type declaration to really disable sendable checking.

Apologies for the late reply here, @John_McCall!

Wanted to prepared a detailed writeup but then got pulled into some work stuff... :sweat_smile:

Right, worth being more specific here. You got the use-case right: "run specific work off the default pool, and instead on some other specified pool" is the gist of it.

While perhaps worth a separate thread (maybe we can split this out if worth discussing right now? or wait out until we're ready to deep dive into those). I wanted to write up some of what would help addressing this problem though, even if just as food-for-thought:


1. First, one approach here is to make specific actor run on specific thread pool, by overriding the unownedExecutor.

// Now, I'll admit that this is a bit unusual (to me at least, my brain still very used to how akka does this) in Swift actor's current shape where the actor gets the serial execution semantics from the executor: the executor it has must be a SerialExecutor and that's where we get the serial actor execution semantics from.
I'm a bit more used to treating the "executor" (or "dispatcher") as a "dumb" thread pool and the actor's implementation making sure it only submits "run X now" to the dumb pool/dispatcher/executor which just runs things that are given to it. But I'll do my best to stick to Swift wording for consistency!

One approach that helps here is to allow specific actors to declare where they should run all their tasks. This is not unlike Swift's actors and the main actor which happens to use the main thread.

// let's say, in my system I want to dedicate 4 threads (made up number)
// to blocking tasks; and I'll want to make sure all blocking work is done
// on those, rather than on the global pool:
let dedicatedToBlockingTasksPoo = ThreadPoolExecutor(threads: 4)
// This is NOT a SerialExecutor, it is an Executor though:
//     public protocol Executor: AnyObject, Sendable {
//         func enqueue(_ job: UnownedJob)
//     }

Next, I can have any number of actors which I know will be doing blocking, and I make them all use this pool:

actor Blocker { 
  let resource: Resource
  
  let unownedExecutor: UnownedSerialExecutor
  // overrides: 
  // nonisolated var unownedExecutor: UnownedSerialExecutor { get }

  init(r: Resource, executor: ThreadPoolExecutor) { 
    self.resource = r 
    self.unownedExecutor = executor.unownedSerialExecutor // "give me an actor (Serial) executor onto this thread pool"
  }

  // async, but we'll do this work on the blocking executor, good.
  func blockingWorkOnResource() async { <do blocking (e.g. IO) on Resource> }
}

So this is nice, because all methods on this actor would hop to the dedicated pool, and it can do whatever kind of blocking on the resource it needs to do, without blocking the shared global width-limited pool.

This is exactly "the akka way"; where we'd tell people to dedicate some threads for their IO, and run their e.g. database calls on such. In Scala/Akka, the "dispatcher" can be used for either Futures, or actors, since it's just a thread pool, the pattern looks like this:

// config file
my-blocking-dispatcher {
  type = Dispatcher
  executor = "thread-pool-executor"
  thread-pool-executor {
    fixed-pool-size = 16
  }
  throughput = 1
}

Which is then given to actors:

// scala / akka, just to examplify the pattern:
context.spawn(<my actor>, "name", .fromConfig("my-blocking-dispatcher"))

// this is equivalent to in Swift:
<MyActor>("name", <the dispatcher>)

But also they can be passed to Futures (so equivalents of our Task {}):

// scala / akka, just to examplify the pattern:
implicit val blockingDispatcher = system.dispatchers.lookup("my-blocking-dispatcher")

Future { Thread.sleep(10000) } // uses blockingDispatcher implicitly
// equivalent to:
// Future({ Thread.sleep(10000) }) (blockingDispatcher)
// https://www.scala-lang.org/api/2.13.x/scala/concurrent/Future.html

Which brings us to the second part, running specific tasks on specific pools:

2. Sometimes it is useful to run a specific task on a specific pool / executor, without having to go all the way to make an actor for it.

Though perhaps we could say we don't do that in Swift, and instead resort to global actors, and one would declare a global MyIOActor, give it the executor like we did in 1. and call it a day? Then we could:

Task { @MyIOActor in blocking() }

which could be nice as well...?

Otherwise, an approach would be to pass an executor to a Task:

Task(runOn: myBlockingExecutor) { ... }

The global actor approach may be worth exploring though...

3. And since we want to push developers to use child tasks whenever possible, they may also need this ability.

In TaskGroups the API can be rather easily extended to provide "where to run the child tasks":

TaskGroup(of: Int.self, childTaskExecutor: myBlockingExecutor) { group in 
  group.addTask { /*uses myBlockingExecutor*/ }

  group.addTask(on: otherExecutor) { /*uses otherExecutor*/ }
}

So we could have this way to kick off some blocking tasks in a structured fashion.

It gets more complicated with async let, where we probably would need to resort to some scope pattern?

Executors.startChildTasks(on: blockingExecutor) {  
  async let x = { io() on blockingExecutor }
}

// or we'd have to invent some other way to annotate, maybe ideas like
async let x = { @IOActor in ... } // could be a thing...?

could be one way to achieve this... though we'd likely have to carry this executor preference in a task-local, (or make other space for it).


So overall, it all boils down to having to call some blocking code, in various situations, and making sure this code won't execute on the global pool.

Visually, (taken from an ancient writeup I did over here) we never want to have any blocking work on the "default" pool: (cyan=blocking/sleeping, green=running, orange = waiting), like the following bad situation visualizes:


(default pool is completely saturated by sleeps/blocking)

but instead, we'd want to have a world where all the bad things still are happening, but on their own executor/pool separated from the global pool:


(default pool is not busy at all, ready to serve requests, but blocking pool is busy sleeping blocking). This is better since it allows the server to remain responsive and reply with 500 or timeouts or do whatever else it needs to be doing...

Not sure how helpful the writeup is, but I figured I could collect a bunch of ideas and requests regarding this to give the discussion some more concretes -- though also happy to delay diving deeper until we're ready to discuss custom executors more etc.

10 Likes

Even if this was possible, this could only address the OP's need (blocking a sync function until an async one has completed) provided what you call the "specific work" never awaits on jobs that happen to run on the thread that is blocked by the waiting OP's synchronous function.

I don't see how this can be guaranteed, how it is possible to avoid deadlocks.

Even if one "derails" some tasks with alternative executors, those tasks may await on other async functions that must run their jobs on the default pool, or a specific thread, or use continuations in order to wrap god knows what.

The main thread is the first example that comes to my mind. If I run, from the main thread, a sync function that blocks until a Task ends, then any @MainActor or DispatchQueue.main that happens somewhere during the Task's execution will deadlock.

One could imagine a technique to avoid dealocks: only allow sync functions to wait for an async one when they are started from special threads: threads that are guaranteed to never be used by async functions.

But such functions could not be called from async functions. Such functions could not be called from the main thread either. This would just ruin the OP's intent, which is to define public sync apis that wrap async ones (with the ability to freely call them).

Am I right saying that deadlocks are lurking everywhere in this whole forum thread?

@MainAction func f() {
    // It will be very difficult for the Vapor team to guarantee
    // that their async implementation of `doStuff` NEVER
    // happens to schedule something on the main thread,
    // directly or indirectly through third-party or system libraries.
    let x = Vapor.syncDoStuff() // block and cross fingers
}

The API I've been mulling for non-actor custom executors is just to provide some sort of async API that switches to the executor, like:

extension Executor {
  func run<T>(operation: () async throws -> T) async rethrows -> T
}

The target executor would then be used in place of the default global executor for the current task for the duration of the scope. That is, whenever control entered a non-isolated async function, i.e. the situation where Swift would normally switch to the global executor, it would instead switch to the target executor.

I think the right behavior is for this to also be picked up by structured subtasks and Task {}s created within the scope, but of course not by Task.detached {}.

I'm not sure what should happen if you call this on a serial executor, or if that should just be a precondition failure. Also the name run is possibly poorly-chosen.

2 Likes

I doit see the interest in letting the OP, and other readers of their thread, in a state of expectation.

And should I dare : not answering reasonable doubts (sorry for taking the time to write them down above) is also a strategy I can not understand.

This was the only reasonable (from which one can reason) that was provided so far, and no single people in charge of Swift concurrency has provided any answer to this.THIS is also something I can not understand.

The Vapor team is in dire straits, this is something I can understand. But hand-waving answers is not a correct answer to maintainers of such an important library.

1 Like

I've read the thread carefully but I can't see it: can you clarify what question is being ignored or hand-waved away?

So far as I can see the original Vapor ask was answered up-thread.

1 Like

I guess I have been awkward in my wording, sorry for that. It looks to me that the answer to the initial Vapor ask was "sure, you can do it, with unsafe features". But considering the current uncertainty of the Vapor team regarding their thread-safety, I would expect the initial optimistic answer to be tampered. I'm wary that @0xTim thinks that blocking a thread until an async function completes is a good idea - when all I can see are opportunities for deadlocks, and just more uncertainty on top of the current one.

To be clear, this is the only way for Vapor to achieve their stated goal of keeping their old synchronous APIs around while basing them on the new asynchronous ones. While that's unfortunate, it's not possible to design this away, because it's inherent in the problem. A synchronous operation must block the calling thread until the operation is complete, and an asynchronous operation may delay execution.

The only ways out of this mess are:

  1. Remove the synchronous API
  2. Provide two separate implementations
  3. Drop actors and use locks directly, at the risk of a much less safe implementation

But there's nothing the Swift concurrency team can do to make this easier: the complexity is inherent in the problem domain.

4 Likes

I looked at the vapor pull request and it seems actors are mostly used here to gate access to simple dictionaries, that are then used in many places. I wonder:

  • is it worth the cost of going async for doing something as trivial as accessing a dictionary?
  • the actors are used in many places and cause a cascade of async contamination (the infamous coloring problem), this might contribute to the other infamous problem of interleaved code execution, is the use of such fine-grained actors really the right design?
  • let's be honest, it's not like using locks to gate access to dictionaries would be a difficult thing to do and produce much more unsafe code. I could even argue that avoiding the interleaved code execution problem produces safer code.

I understand the appeal for using actors, but is it really worth the cost in this particular case?

1 Like

If this is indeed merely for protecting array / dictionaries, using mutex lock or an equivalent is more than adequate and probably the fastest possible solution. It would be very hard (albeit possible *) to get into a deadlock situation in code like this:

func value(forKey: Key) -> Value? {
    mutex.lock()
    let value = dictionary[key]
    mutex.unlock()
    return value
}

func setValue(value: Value, forKey: Key) {
    mutex.lock()
    dictionary[key] = value
    mutex.unlock()
}

(*) unless there's some exotic scenario, like a custom EQ/hash key implementation that might in turn access a data structure that needs locking.

OTOH, rarely atomic protection of individual data structures is enough, more often several data structures need to be protected at once. Plus in real cases it would be more than just the above. Consider sort / filter / find as an example, where you need to supply a sorting / filtering / searching closure - and there's nothing exotic in that - the closure that might in turn touch other data structures that need protection. One "hammer" approach (though not optimal) to this - sort / filter, etc the copies, and change the original data structure atomically (like in the above get / set).

To clear up a few comments that have come up:

  • We're not discounting any implementation - our stated goals of safety and Sendable adoption in a non-breaking way are the goals. If there is a better way to do it other than the current approach then we'll take it
  • This is a very real possibility - leave the existing internals as they are (with probably the addition of locks for thread safety) and heavily nudge users to use the new APIs.
  • The current PR only has fairly simple stuff. However it's only going to get more complex as more internals get reworked. I don't want to get in the situation where we stick with something like looks and a) ship a deadlock or unsafe code that would have been caught by a compiler or b) box ourselves in for any required future changes.

We want to make use of Swift features, especially those that ensure both Vapor and Vapor's users write safer code with more errors pushed from runtime to compile time

This seems the safest choice. Also, it doesn't seem like more work since you are leaving your current API as-is and you have to implement the new one anyway.

I'm sad to see that the Swift concurrency story has us cornered into fear of using anything other than actors. Regardless, many awaits have been added to the code in places that just didn't have them before. Code that used to execute in one go may now suspend and execute re-entrantly, now I'm genuinely curious, did you audit the code and were able to confirm that this is a non-issue and the code is safe?

Have you tried the design I suggested upthread?

It seems to me that it is essentially what @ktoso is describing - running the operations themselves on a non-pooled thread, and having Swift concurrency callers enqueue their work with a continuation so you never block resources from the global pool.

Of course, you'll have your own prioritisation for the things you want to deliver as part of this change. Personally, thread safety would be my top priority, followed by exposing non-blocking async calls, followed by deadlock prevention as a "nice to have". Non-thread-safe code being accessed from multiple threads means undefined behaviour, and fixing that would be independently useful even if Vapor didn't use any other Swift concurrency features.

Those non-thread-safe resources will need some kind of mutual exclusion to serialise accesses (that's inherent to the problem, as has been mentioned), and IMO the most natural fit for a system which should eventually be based on language-integrated actors would be to use a dispatch queue. There are clear parallels between the private queue and the actor's internal "message box". Dispatch queues are also the primary way pre-Swift-concurrency async code is implemented today on Apple's platforms and elsewhere, so it's a familiar pattern to many Swift developers (which can be useful as these features roll out and perhaps bugs emerge which the community will want to help fix).

I believe that actors are a good basic design tool and the right way to structure the high-level concurrency in many systems — if you're exposing a type with a concurrent interface, I think you should be strongly considering making that type an actor. But there are definitely still a lot of uses for synchronous locks, especially in the implementation of systems that intend to largely provide synchronous interfaces. More importantly, Swift will eventually have some sort of native lock type; we've focused on actors for good reasons, but I don't want people to avoid locks just because actors feel more native right now. If you have a well-designed lock type (like Apple's OSAllocatedUnfairLock, although I personally would like that a lot more if it really committed and forced you to use the withLock API that provides the state inout), you should feel free to use it fearlessly. Locks are less composable tools than actors in some ways, and you generally need to be more thoughtful about what work you do while holding them, but the guarantee of synchronous behavior is a very powerful tool that's sometimes just what you need.

(And there's a more general point there; good concurrency design includes understanding when to deliberately make things synchronous, e.g. in order to force them to happen uninterrupted, or to force clients to "come prepared".)

I'm not trying to pass judgment on the APIs in question here; I haven't looked at them at all. If there's a good reason for them to become async, then absolutely, make them async. I just wanted to clarify how I, personally, think about how actors fit into a design and when to use sync/async functions.

13 Likes