1

I am using a task group to wrap repeated calls to a long running async method. Since I may have many calls to this method that need to happen the hope is that they would run in parallel. At the end of the day I have some regular synchronous code that needs to block until all of these asyncs are called.

What's weird is unlike other examples of task groups I have seen I actually do not need the values from the async throws method. They do actions and write them to disk.

Is there not a way to clean up this code?

let swiftAsyncSemaphor = DispatchSemaphore(value: 0)
let taskGroupTask = Task {
    await withThrowingTaskGroup(of: ASRJob.self) { group in
        for path in filePathsToWorkOn {
            group.addTask {
                return try await doHardWork(atPath: path)
            }
        }
        
        do {
            for try await _ in group {
                // For now we do not store this information
            }
        } catch {
            // For now we do not store the error
        }
    }
    swiftAsyncSemaphor.signal()
}
swiftAsyncSemaphor.wait()

Most examples I see use a map/combine function at the end on the group but I don't need the data so how can I just await for all of them to finish?

Context on where this code will run: This code is run within the main() block of a Synchronous Operation (NSOperation) that goes within an OperationQueue. So the block calling wait is not in swift async code. The goal is to block the Operations completion until this swift async work is done knowing that it may very well run for a long time.

CalebK
  • 353
  • 2
  • 11
  • 2
    (Maybe) unrelated but do not use semaphores with Swift Concurrency. Never! – vadian Jul 27 '23 at 16:06
  • Either use the old Dispatch options or the new Concurrency, your code will up when you choose one. The line following a task that is being “await” will not run until all the “async” code is done. – lorem ipsum Jul 27 '23 at 16:12
  • Ok question. I know having semaphore.wait() inside of swift async code is a big no no. Why is .signal() a bad thing? @vadian – CalebK Jul 27 '23 at 16:50
  • Technically, the `signal` is not the problem. The `wait` is. It has always been an antipattern (at best, it is inefficient because it unnecessarily blocks threads; at worst, it can lead to watchdog terminations, deadlocks, etc.). We would generally use dispatch groups (using `notify`, not `wait`), completion handlers, etc., in pre-`async`-`await` codebases. But it is explicitly prohibited in Swift concurrency. – Rob Jul 27 '23 at 17:08
  • You are mixing the concepts of threads and actors, actors are much safer and much more organized when used properly. The new concurrency uses actors. – lorem ipsum Jul 27 '23 at 17:13
  • Really appreciate this help. Been having troubles finding information on these details. Updated question with more context (@Rob) – CalebK Jul 27 '23 at 17:25
  • Confusion here. I thought blocking an operation on the wait() would be ok. Technically that thread is a bit wasteful but can't it get suspended and moved into the schedulers queue until the semaphore fires? Or is it going to block up an entire physical core. – CalebK Jul 27 '23 at 17:26
  • Blocking the thread for an `Operation` is not advised, either. Best practice is a custom concurrent subclass of `Operation` (avoiding ever blocking a thread). See [Trying to Understand Asynchronous Operation Subclass](https://stackoverflow.com/q/43561169/1271826) or see the “concurrent operation” discussion in the `Operation` [documentation](https://developer.apple.com/documentation/foundation/operation/). This is why so many of us were thrilled to rip out all that `Operation` code and move to Swift concurrency, as it handles dependencies between asynchronous tasks so much more gracefully. – Rob Jul 27 '23 at 17:31
  • Yeah completely on this boat. So I originally had this code written in an asynchronous Operation and it worked great. But I couldn't understand why I should expect it run differently than the semaphore version so I simplified it. One blocks the synchronous operation on calling finish to signal that the Operation is finished. The other blocks the synchronous operation on signaling a semaphore. Why would they be different? – CalebK Jul 27 '23 at 17:35
  • Because one blocks a thread and the other doesn't. You only have 64 threads in that worker thread pool. It's one of those things that everything looks honky-dory in most situations, but can introduce hitches, deadlocks, etc., when the system is taxed. It's why they introduced that complicated KVO-based concurrent `Operation` system in the first place. (And it's why we all love `async`-`await`, eliminating all that complexity.) – Rob Jul 27 '23 at 17:49
  • Ok so the issue is more there are a finite number of worker threads vs there being “good” blocked threads and “bad” blocked threads? – CalebK Jul 27 '23 at 18:56
  • Correct, it’s not good vs. bad, but rather that they’re limited, so we should avoid ever unnecessarily block them with wait. It's also that if you never block, it eliminates most deadlock risks. (It’s also why we avoid “thread explosion” scenarios with unbridled tying of of threads. When we do need lots of threads, we use things like GCD concurrentPerform or async-await cooperative thread pools, both of which are constrained to the number of CPU cores. But this is all a separate topic.) – Rob Aug 03 '23 at 15:49

1 Answers1

3

If you use semaphore.wait() from within an asynchronous context, it will produce this warning:

Instance method 'wait' is unavailable from asynchronous contexts; Await a Task handle instead; this is an error in Swift 6

I assume that you must be waiting for this semaphore outside of a asynchronous context, otherwise it would have produced the above warning.


You cannot use semaphores for dependencies in conjunction between async-await tasks. See Swift concurrency: Behind the scenes:

[Primitives] like semaphores ... are unsafe to use with Swift concurrency. This is because they hide dependency information from the Swift runtime, but introduce a dependency in execution in your code. Since the runtime is unaware of this dependency, it cannot make the right scheduling decisions and resolve them. In particular, do not use primitives that create unstructured tasks and then retroactively introduce a dependency across task boundaries by using a semaphore or an unsafe primitive. Such a code pattern means that a thread can block indefinitely against the semaphore until another thread is able to unblock it. This violates the runtime contract of forward progress for threads.


The pattern in async methods would be to await the result of the task group.

let task = Task {
    try await withThrowingTaskGroup(of: ASRJob.self) { group in
        for path in filePathsToWorkOn {
            group.addTask {
                try await doHardWork(atPath: path)
            }
        }

        try await group.waitForAll()
    }
}

_ = await task.result

For throwing tasks groups, we would also often waitForAll.


Yes, you can technically wait in an Operation, but even that is an anti-pattern. We would generally write a “concurrent” operation. See discussion of “concurrent operations” in the Operation documentation. Or see Trying to Understand Asynchronous Operation Subclass.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • Really appreciate the detail here. I am a bit confused though I thought per reading those guidelines you posted the only illegal thing would be to call semaphore.wait() within the swift async code. Semaphore signal should be fine right()? – CalebK Jul 27 '23 at 17:29
  • I am trying to do a partial adoption of swift async and while it isn't ideal I am just looking for the most "legal" way to block old synchronous code on the completion of new swift async code. – CalebK Jul 27 '23 at 17:29
  • As discussed above, best practice would be to write a custom “concurrent” `Operation` subclass, thereby eliminating the need to `wait` at all. Best would be to bite the bullet and eliminate operation queues, entirely. Technically, you could put the semaphore signal after `group.waitForAll()` and then `semaphore.wait()` in lieu of `await task.result`. But, I would strenuously advise against it. Why use Swift concurrency if you're just going to entangle it with legacy anti-patterns? – Rob Jul 27 '23 at 17:45
  • Ok thanks for the advice. Will be wrapping it in an async operation – CalebK Jul 28 '23 at 02:14