What's the correct way to spawn a Task in an Operation?

Using Swift
,
1

I'm currently refactoring a codebase to use async-await. Previously, network requests were made blocking the thread they were called from, and at the top level any functions that started network requests were called within DispatchQueue.global(qos: ...).async { ... }. However, there is one exception I'm unsure of how to handle.

I've come across an OperationQueue full of a specific subclass of Operation. It takes two function objects in its initializer -- one to make the network request, and a completion handler -- and effectively does this:

override func main() {
  if isCancelled { return }
  let results = makeNetworkRequest()
  if isCancelled { return }
  DispatchQueue.main.async {
    completionHandler(results)
  }
}

Refactoring this to use a task is harder than it seems on first glance. I've marked the completion handler as @MainActor, and added a piece of internal state to hold the task:

enum TaskState {
    case notStarted
    case executing(Task<Void, Never>)
    case finished
}

private var taskState: TaskState

Then, I initially wrote these overrides (KVO compliance omitted for brevity):

override var isFinished: Bool {
    if case .finished = taskState { true } else { false }
}

override var isExecuting: Bool {
    if case .executing(_) = taskState { true } else { false }
}

override var isAsynchronous: Bool { true }

override func start() {
    guard !isCancelled else { return }

    self.taskState = .executing(Task.detached(priority: /* ... */) { [weak self] in
        defer {
            if let self {
                self.taskState = .finished
            }
        }
        
        if Task.isCancelled { return }
        
        if let results = await self?.makeNetworkRequest() {
            if Task.isCancelled { return }
            
            await self?.completionHandler(results)
        }
    })
}

override func cancel() {
    super.cancel()
    if case .executing(let task) = taskState {
        task.cancel()
    }
    self.taskState = .finished
}

Do you see the problem here? It's very subtle.

Operation is @unchecked Sendable, so it needs to be thread-safe. start() and cancel() may race. In particular, if the order of events is

  • start() checks isCancelled
  • cancel() runs in its entirety
  • start() sets taskState = .executing(...)

then the cancellation isn't respected.

I need to serialize start() and cancel() in some way. What's the correct approach here? Some thoughts I've had include:

  • I probably need to add guard case .notStarted = taskState else { return } to start (between the two existing statements), along with...
  • Wrapping the parts of cancel() and start() that touch taskState (and not the guard !isCancelled / super.cancel()) in something like a mutex -- but that has the potential to block the thread calling cancel(), which could be the main thread.
  • Or it could be a serial DispatchQueue, which uses sync in start() and async in cancel()
  • Or maybe this is a solved problem I'm just not aware of the solution to?
2

I suppose the first thing you need to make is to protect taskState with a mutex. Since both methods can run from different threads, its mutation is unsafe.

3

Honestly, if you can, you might be better off throwing out operation queue and firing off tasks yourself and managing them through a task group. But if you can't do that, then like @vns said you'll need a mutex (like a DispatchQueue or OSAllocatedUnfairLock) to protect mutation of your state and calls to start and cancel.

One issue that comes up there is that you're trying to synchronize the state of an asynchronous, concurrent Task with code that isn't aware of Swift's concurrency model, which comes with some risks — they can easily step on each other's toes and open you up to deadlocks, and getting thread-safe implementations of Operation right is hard to begin with.

2 Likes
4

I can look into this, but at a glance I don't believe this is feasible for two reasons:

  • OperationQueue manages dependencies automatically; TaskGroup starts its children immediately
  • In our case, the OperationQueue is kept alive well past the function that starts it and more operations may be added dynamically based on user interaction. withTaskGroup provides far too narrow a scope.

Yeah, I'm looking for guidance on what the best solution is. We only just got approval to bump the deployment target past iOS 12 -- hence why the refactor didn't happen sooner -- and there's no way we're going all the way to 16 for OSAllocatedUnfairLock. So my options are basically DispatchQueue and NSLock (and maybe an actor?) and I'm asking what the best approach is.

You're telling me...

5

I'm realizing now that there's secretly a third mutation of taskState:

That also needs to be synchronized, and given that you aren't supposed to block the global concurrent executor's thread pool, that probably can't be any kind of traditional (blocking) mutex. Is DispatchQueue the only option?

6

There are some libraries (1, 2) that try to basically create an implementation of OperationQueue but for Swift concurrency (I'd guess there are others besides those I linked.) You may want to try those out if you're opposed to writing your own.

1 Like
7

So long as this is the last thing that runs in your task, and the update doesn't happen synchronously, I think you'd probably be fine with firing off a DispatchQueue.async { ... }...?

1 Like
8

Thanks for the links, I'll definitely check those out tomorrow!

If I do end up being stuck with Operation + DispatchQueue,

  • Am I right in assuming that start() should use sync { ... } rather than async { ... } to avoid the overhead of switching threads?
  • Does the task need to be detached, or can/should it be a normal Task?
9

Task almost never needs to be detached. Use just regular Task as default. Task.detached is equal of using DISPATCH_BLOCK_DETACHED flag in GCD (of which I wasn't aware until Swift introduced this in Task). But this looses priority, task locals, etc.

I would prefer synchronous operation body, it is usually easier. Since operation queue manages all separately from Swift Concurrency, you can synchronize your async operations if needed with quite of a confidence.

Plus, note that in either way – be it Task or DispatchQueue – you have the initial problem of start and cancel possibly called on separate threads, therefore you would need to synchronize access to state and check for "re-entrancy".

You generally want to design it so that missed cancellation isn't that critical otherwise – because there is just no way you'll be able to deterministically define that.

Note from the side thread, I see now why you don't have an access to the OSAllocatedUnfairLock. You can look up a similar implementation, the most practical one would be Protected from Alamofire and use in the same manner. But in general even just NSLock is definitely available and solves the task.

4 Likes