1

Hello.
In some protocol requirements, and when dealing with some old APIs such as AVFoundation, it may come to the edge case where you will have to use an async function in a sync context.

Here is my implementation, and I would like to ask some questions.

import Dispatch

func withAsyncResult<T>(builder: @escaping () async -> T) -> T {
    let semaphore = DispatchSemaphore(value: 0)
    let pointer = UnsafeMutablePointer<T>.allocate(capacity: 1)
    Task {
        await pointer.initialize(to: builder())
        semaphore.signal()
    }
    semaphore.wait()
    return pointer.pointee
}

The code runs fine, for example

class Model {
    
    var a: Int? = nil
    
    func update() {
        self.a = withAsyncResult {
            try! await Task.sleep(for: .seconds(1))
            return 1
        }
    }
    
}

let date = Date()
let model = Model()
model.update()
print(model.a)
print(date.distance(to: Date()))

would print

Optional(1)
1.045375943183899

Is this a valid implementation? Will this implementation ever hang the main thread?
Yes, there is a compiler warning of Capture of 'pointer' with non-sendable type 'UnsafeMutablePointer<T>' in a @Sendable closure, but all it ever introduce data racing? The semaphore should ensure the task is completed before returning.

If it does work, how is that different to this?

@MainActor
class Model {
    
    var a: Int? = nil
    
    nonisolated func update() async {
        Task { @MainActor in
            try! await Task.sleep(for: .seconds(1))
            self.a = 1
        }
    }
    
}

let model = Model()
await model.update()
try await Task.sleep(for: .seconds(2))
print(model.a)

Which one is more efficient or error-prone? why?

Any suggestions would be greatly appreciated. Thank you in advance!

I understand the examples are quite weird, but in my practices of writing SwiftUI Buttons and need to call async functions, I would do something like this:

Button {
    Task {
        self.state = await foo()
    }
}

Which is somewhat similar to the example.

Then with the new function, I could:

Button {
     self.state = withAsyncResult(foo)
}

Seems to me the second one is clearer.

2

Use of DispatchSemaphore is a bad idea (or any other form of blocking from old GCD patterns), because Swift Concurrency operates on a limited number of threads, so you can simply run into a deadlock. So even though it does work right now during your launches, it is not safe.

I suppose what you are looking for is the following:

@MainActor
final class Model {
    var a: Int? = nil
    
    func update() async throws {
        await doWork()  // this will run on a generic executor
        a = 1
    }

    // nonisolated will make it call outside of main actor
    nonisolated func doWork() async throws {
        try await Task.sleep(for: .seconds(1))
    }
}

The first example is just correct. Your sync version will simply block the thread. Mark the whole view isolated on main actor as well, so your Task inside button's action will run on the main actor.

@MainActor
struct ModelView: View {
    var view: some View {
        Button(action: { 
            Task {
                try await model.update()
                state = model.a
            }
        }, label: { Text("Update") })
    }
}
3

Important to note that this snippet disregards cancellation and error handling completely, which is a bad user experience:

That's why use of Task.init is so dangerous and in general is discouraged in favor of structured concurrency. If there is no other way (e.g. you could maintain an AsyncSequence of events that synchronous context would use for yielding to async context instead), at least make sure you don't ignore errors.

As a quick hack, this somewhat better (but still suboptimal) updated snippet makes it impossible for users to cancel ongoing tasks and doesn't guard model updates from reentrancy if the user taps the button multiple times in succession. But if something went wrong, your model can be notified of the failure:

@MainActor
struct ModelView: View {
    var view: some View {
        Button(action: { 
            Task {
                do {
                    try await model.update()
                    state = .success(model.a)
                } catch {
                    state = .failure(error)
                }
            }
        }, label: { Text("Update") })
    }
}
1 Like
4

That's a good note on error handling, probably shouldn't ignore it myself in example as well.

SwiftUI has made it a bit complex to not use Task inside callbacks in views - it does not come out of a box, and AsyncStream support is not a trivial way. If making an example complete from my view, I would incapsulate error handling at the model side:

@MainActor
final class Model: ObservableModel {
    @Published
    var lastError: Error?

    func update() async {
        do {
            try await doWork()
            a = 1
        } catch {
            // handle error here, 
            // propagate to UI via @Published property, for example
            lastError = error
        }
    }
}
5

Consider doing neither of this: don't have "state" in the view, have in the model: all of the sudden you no longer need Task or await, etc in the view code.

6

Agree to move state completely to the model, yet exposing 'synchronous' only at first sight interface within it and wrapping everything inside model into a Task doesn't look like a good idea.

7

Could you elaborate on that please?

8

Please consider this model:

@Observable
final class Model {
    var state = 1
    
    func update() async {
        self.state += await foo()
    }
}

How is it possible to call update without using a Task?

9

I am wondering if this is the best way to handle errors, capturing the error instead of throwing it all the way up. I would usually mark update as async throws, and capture the error in the Task calling this method. That would involve making another state inside the view of course, so your solution should be better than mine.

10

Task is required, unless you change the foo above to be sync, e.g.:

class Model: ObservableObject {
    @Published var state = 1
    func foo() {
        DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
            self.state += 1
        }
    }
    func update() { foo() }
}
11

I cannot say it is the best, I just prefer it. Why? Mostly because in that way error handling stays within the model itself, and view can receive just some state, where error might be transformed in some way. And if I use model with other view error handling still in the model. It has some drawbacks that now you have to manage correct error passing to the view, for example.

The error handling strategy will (and probably should) vary in general. So I wouldn't say that always passing errors all the way to the top is necessary. Just don't disregard them and handle meaningfully. And with Task there is another danger, that it swallows errors, while at every other level compiler will help you to remember about errors handling (that's actually another reason why I decided to keep errors in the model once started adopting new concurrency).

1 Like
12

SwiftLint actually has check for Task usage with unhandled error: unhandled_throwing_task Reference. It's not enabled by default. I think that is a great addition.

2 Likes
13

Using semaphore to force to wait for the Task completion is very risky and I wouldn't use it in the production code.

There is a great blog post about this topic in detail Swift Concurrency Waits for No One

Stick to other well-known tools for that.

For the Button action case. Personally I prefer to push as long a possible up the chain the need of spinning unstructured Task. In this case I would rather make a view model with the async function and spin a task from the View

Button {
    Task {
        await viewModel.buttonTapped()
    }
}

If you need to prevent the re-entrancy you might handle it either by making a guard in
viewModel (e.g. checking the state or by keeping a reference to a task)

You might also create a custom AsyncButton reusable component that would have built-in reentrancy prevention + async actions support.

I agree that SwiftUI didn't encounter proper adoption of Swift Concurrency (besides the task modifier), leading to many (sometimes improper) solutions to the lack of handling async/await by default.

Testing unstructured Task is also a non-trivial topic

3 Likes
14

Fundamentally, all data which the UI needs should be isolated to the main actor.

Your model (including ObservableObjects) should be @MainActor-isolated, and using a nonisolated which just dispatches to the main actor introduces unnecessary delays which makes your code harder to reason about.

When the framework invokes your body property or you get callbacks from a Button or something, that all occurs on the main actor, and those operations need to be able to read or modify that data synchronously. When you do that, no awaits are required (in theory; the SDK still has gaps in its concurrency support - but that doesn't mean you should do the wrong thing! There are workarounds...).

I've encountered a lot of developers who have misconceptions about Swift's language-integrated concurrency features, and what they can bring to their UI applications. It won't magically make your code scale better on multi-core machines, and it does not mean you should "async all the things" or chuck a bunch of Tasks everywhere. What it does is it allows you use the same kinds of concurrency features that you're used to from GCD or other concurrency libraries, but with added confidence that you are not introducing low-level data races, and with better support for Swift's control flow (such as throwing errors).

For instance, most developers are aware that network requests should occur on some non-main thread, as they can take arbitrarily long. Using GCD, they might write something like this:

var modelData: [Model] = ...

func refresh() {
  DispatchQueue.global().async {
    modelData = downloadModelData()
  }
}

class View {
  // reads modelData
}

The problem here is subtle - the view (which lives on the main thread) accesses modelData synchronously, as it must to display its contents. But then, within refresh, we modify modelData from a different thread. Simultaneous reading and assignment to the same memory location is a low-level data race.

Previously, some of these errors could be caught at runtime by the "Main Thread Checker". With Swift Concurrency and @MainActor annotations, all of these races will be caught, and they'll be caught at compile-time instead. I would say that is like... probably 90% of the benefit that authors of UI code will get from Swift concurrency.

@MainActor
var modelData: [Model] = ...

func refresh() async {
  modelData = await downloadModelData()
//╰─ error: main actor-isolated var 'modelData' can not be mutated from a non-isolated context
}

func refresh() async {
  let newData = await downloadModelData()
  await MainActor.run { modelData = newData }
  // OK 👌
}

And then at the top level, where some user interaction prompts the refresh operation to be launched, that's where you want to introduce an unstructured task, because refreshing model data is something happens concurrently with the view's normal operation, and so it makes sense for it to be async from the perspective of the view.

Button {
  Task.detached { await refresh() }
}

And that last sentence is important: things which are async from the view's perspective are things which make sense to occur concurrently with its normal operation. On the other hand, things like this:

Button {
    Task {
        await viewModel.buttonTapped()
    }
}

suggest to me that the modelling is probably not quite correct. Reacting to a button tap is not something that happens concurrently with a view's normal operation -- it is the view's normal operation! It should be able to read and modify the view's state synchronously, which means buttonTapped should be synchronous and isolated to the main actor (or in the unlikely case that it touches no view-relevant state, it may be non-isolated).

To put it another way, if you're familiar with Obj-C, an async buttonTapped method is essentially equivalent to this:

func buttonTapped(completionHandler: @escaping () -> Void) {
  DispatchQueue.global().async {
    ...
    completionHandler()
  }
}

Most applications don't do things like that for simple button handlers; it's too much asynchronicity and makes your code harder to reason about by introducing unnecessary delays in to your code.

7 Likes
15

This might be an anti-pattern... See below.

There was a dispute on the team where one team member suggested that this load should be done from background:

// starts on the main queue
// completes on the main queue
func load(url: URL, execute: @escaping (Data?, Error?) -> Void) {
    dispatchPrecondition(condition: .onQueue(.main))
    // Hint: the "delegateQueue: .main" is used here to avoid the subsequent 
    // explicit jump to the main queue where the result is expected.
    let session = URLSession(configuration: .default, delegate: nil, delegateQueue: .main)
    session.dataTask(with: URLRequest(url: url)) { data, respone, error in
        dispatchPrecondition(condition: .onQueue(.main))
        execute(data, error)
    }.resume()
}

while another opposed that it doesn't matter that this requests starts and completes on the main queue: the actual networking is done internally on a background thread.

Where is the truth? I tend to agree with the latter viewpoint but I don't know the amount of overhead that happens on the main thread when the dataTask is scheduled from the main thread.

Considering the latter viewpoint is correct and disregarding the pathological ancient synchronous API's like Data(contentsOf: url) (which should be deprecated IMHO) I don't see how to use "the network requests should occur on some non-main thread" suggestion in practice. It could be even an anti-pattern where developer would introduce an explicit extra hop-on to a background queue to execute a dataTask, something which would complicate the code and not do anything useful.

1 Like
16

You are misunderstanding; I said the network request should occur on a non-main thread. This observation is independent of any particular API, and applies to other kinds of long-running operations as well, including filesystem IO.

In the case of Foundation's URLSession API, this has been built in to its design; you may submit requests to the session from any thread (including the main thread), and the session will execute those requests on some private thread. There is no contradiction here.

In the examples above, you can implement the downloadModelData() function using URLSession's new async APIs. You would get the same benefit that before you can mutate data which is shared with the UI, you would need to ensure that you are back on the main actor. So it statically eliminates what would otherwise be a low-level data race.

@MainActor
var modelData: [Model] = ...

func refresh() async {
  modelData = await downloadModelData()
//╰─ error: main actor-isolated var 'modelData' can not be mutated from a non-isolated context
}

func downloadModelData() async -> [Model] {
  let (data, response) = await URLSession.shared.data(from: URL(...))
  ...
}

Conceptually, this is similar to what the "Main Thread Checker" does when you call a participating Apple SDK API, but now it works pervasively across the program and catches mistakes at compile-time instead of run-time.

2 Likes
17

I hear you, just looking at it from slightly different angle. Consider two API's: Data(contentsOf: url) and URLSession.dataTask. Both API's might well occur (do the the actual I/O) on some secondary (internal) thread but it doesn't help the user of the first API – the call blocks the current thread (be it main or not main). I'd say to be a good citizen the I/O operation should not block the current thread (and ideally it should not block any thread). You could have a purely single process and single threaded system and still do I/O in a non blocking manner; on such a system the wording "the network request should occur on a non-main thread" would be not applicable as there's just one thread.