Swift Concurrency: Feedback Wanted!

Evolution Discussion
96

Thank you for this incredible achievement. I do feel actor reentrancy may become problematic when “obvious” extensions to basic actor use cases does not work. A good example is what was demonstrated in the WWDC video: an attempted improvement to caching causes a subtle bug. The fix itself — to add another type of state (the handle) — seems to put the high-level comfort of the actors on shaky ground; your reasoning must now interleave alongside the execution. “Breaking the illusion” to borrow Dave Abrahams’s phrasing

Could there be a way for the compiler to assist in this? E.g. once you add an await to your function, the properties in the actor now require you to consider them in terms of handles. Basically compiler assistance to guide you to the solution described in the WWDC video

2 Likes
97

I also feel uneasy about support for reentrancy but as I understand it, it's a trade off. Without reentrancy, the concurrency runtime would have to spawn arbitrary numbers of threads in order to guarantee forward progress, which could lead to thread explosions. With the fixed set of threads that the default runtime manages you need reentrancy to provide this guarantee. It should ultimately be the faster approach (I believe) but with potential for subtle bugs. Hopefully, as you say, the compiler will soon be able to help out.

1 Like
98

Actors conforming to protocols is still pretty awkward, especially for protocols with defaulted implementations, like Hashable. Implementing hash(into:) is fairly straightforward but doesn't let the default implementation of hashValue work correctly, leading to a compiler error on a property I don't even implement. Implementing both works, but it's hard to say whether it's correct.

nonisolated func hash(into hasher: inout Hasher) {
    hasher.combine(id)
}

nonisolated
var hashValue: Int {
    var hasher = Hasher()
    hasher.combine(id)
    return hasher.finalize()
}
2 Likes
99

As we gain more experience with the actor model, we could probably find common problematic patterns that the compiler or some kind of static analysis tool could diagnose.

Doug

3 Likes
100

That the auto-synthesized hashValue doesn't work is definitely a bug. Synthesizing hash(into:) is actually quite interesting, because you can only sensibly use the let properties to do it.

Doug

2 Likes
101

Just want to start off by saying I'm loving all the code and indentations I'm deleting from my branch as I refactor with Concurrency. Really looking forward to AsyncStream integration with core libraries like CoreLocation, Bluetooth, etc which seems like a match made in heaven.

Now the unfun part.. I'm finding a lot of strange and inconsistent behaviour mixing Combine, GCD, and Concurrency, such as Combine events not triggering, GCD crashing, and Concurrency not continuing. I know a mixed bag isn't ideal, but reasonably unavoidable. Is it possible to create an official document of what can and can't be mixed between these 3 technologies, and if one is required to do so, what are the strategies and nuances we should be aware of?

Here are some sudo samples:

func start() async {
    ...
    cancellable = myCombine.sink {...}
}

Doesn't fire in some scenarios, so seems I have to do this :woozy_face::

func start() async {
    ...
    DispatchQueue.main.async {
        self.cancellable = myCombine.sink {...}
    }
}

And this:

func start() async {
    DispatchQueue.customQueue.sync {...} // Crashes or race conditions sanitizer complaints
}

Or this:

cancellable = myCombine
    .sink {
       async { await start() } } // Continuation isn't reliable
    }

I would love to never use GCD and Combine ever again in favour of Concurrency, but doesn't seem realistic for a another few years to flush it out. Can maybe adding async helpers directly to Combine and GCD help or at least a guideline for a safe approach to mixing them.

1 Like
102

Ideally there should not be any, and we're very interested in hearing more about problems you're running into mixing these technologies. If you're able to file bugs with complete examples of code that isn't doing what you expect, we'd like to take a look and debug what's going on.

6 Likes
103

Those samples all should work; do you have feedbacks or bug reports filed w/ more detailed examples? Combine itself does not really do anything that outlandish w.r.t. threading/queues so it might be a more general bug.

2 Likes
104

With integration into UI, I need cancellation and also like to use result as setting the value for the success and failure is atomic. And often we need to display the result or deal with the error (not ignore it).

So I'm finding myself writing code like this often:

self.taskHandle?.cancel()
self.taskHandle = async {
    do {
        self.result = .success(try await self.search(searchText: value))
    } catch {
        self.result = .failure(error)
    }
}

I'd like to be able to do this:

self.taskHandle?.cancel()
self.taskHandle = async {
    self.result = Result(catching:  { try await self.search(searchText: value) })
}

But I get this error:

Cannot pass function of type '() async throws -> [Place]' to parameter expecting synchronous function type```
105

Result could be extended with an async initialiser — something like this:

extension Result where Failure == Error {
  init(awaiting task: () async throws -> Success) async {
    do {
      self = .success(try await task())
    } catch {
      self = .failure(error)
    }
  }
}

self.taskHandle = async {
  self.result = await Result {
    try await search(searchText: value)
  }
}
5 Likes
106

Thanks, I will do that. In terms of feedback, I guess it would be nice if it was built-in.

2 Likes
107

To be honest, I don't know if this is already reported:
Using the ?? operator does not work without parentheses:

let newTodos = try? await viewModel.refreshTodos() ?? [] // Left side of nil coalescing operator '??' has non-optional type '[Todo]', so the right side is never used
saveNewTodos(todos: newTodos) // Value of optional type '[Todo]?' must be unwrapped to a value of type '[Todo]'

Workaround:

let newTodos = (try? await viewModel.refreshTodos()) ?? [] // no error
saveNewTodos(todos: newTodos)

Expected:

let newTodos = try? await viewModel.refreshTodos() ?? []
saveNewTodos(todos: newTodos)
108

This is correct as-is and has nothing to do with async/await. Without parentheses, try? applies to the rest of the line, producing an optional result, and the left-hand side of the ?? has non-optional type, exactly as the diagnostic tells you.

If you want try? to apply only to the left-hand side of the ?? operator, use parentheses just as you show—this is no different from using parentheses to perform addition before multiplication. It’s not a workaround for some bug, it’s just how operator precedence works.

4 Likes
109

Maybe this isn't the right place to put this but it seems like the concurrency implementation was only partially implemented. Perhaps this is documented elsewhere that some of the functionality is to be implemented later on in the process, but as an example, swift-corelibs-foundation does not provide async/await APIs, which means it cannot be used on URLSession methods outside of Apple platforms.

I had a thread here referencing this How to use async/await w/ docker.

Again, maybe this is something to be expected, but given the push for Swift adoption outside of Apple platforms, I would have thought that this functionality would be able to be experimented with on nightly 5.5 builds/docker images.

2 Likes
110

I've been thinking about this and this solution seems like overkill for my situation. All I need is an @Actor that can guarantee that it's always on the same thread. Maybe @ThreadActor.

111

If we could have a @ThreadActor attribute to guarantee that this actor type is always running on the same thread, then it would work.

112

In the Meet async/await in Swift - WWDC 2021 - Videos - Apple Developer session they mention that we could now use async-await in unit tests, an easy way is to use the XCTAssertNoThrow function to assert that the task didn't throw an error.

Snippet from the session:

    func testFetchThumbnails() async throws {
        XCTAssertNoThrow(try await self.mockViewModel.fetchThumbnail(for: mockID))
    }

I tried something similar but I am receiving this compile error
'async' call in an autoclosure that does not support concurrency

Screen Shot 2021-06-24 at 3.28.18 PM
Screen Shot 2021-06-24 at 3.28.18 PM1475×242 37.5 KB

However, when I wrap the code inside XCTAssertNoThrow with an unstructured task (async), it will work just fine.

it seems that somehow the compiler can't detect that I am in an async function already in unit tests inside XCTAssertNoThrow?

1 Like
113

This is a limitation of the current compiler and XCTest's APIs. XCTest's assertions only take auto closures, which aren't marked async, and so you can't use async APIs in them directly. If the 306 amendment is accepted, XCTest could update with async overloads of the assertions, which we could see in a month or two.

2 Likes
114

I know Combine is an Apple framework, but it would be nice if there was some integration with Combine and async/await. For example, this first async throws getter would be really helpful as a built in async property of Publisher, as I'm sure there are issues with my attempt:

enum AsyncError: Error {
    case valueWasNotEmittedBeforeCompletion
}

class CancellableWrapper {
    var cancellable: AnyCancellable?
}

extension Publisher {
    var first: Output {
        get async throws {
            // Variable tracks if we sent a value or not.
            var didSendValue: Bool = false
            let cancellableWrapper = CancellableWrapper()
            return try await withTaskCancellationHandler {
                cancellableWrapper.cancellable?.cancel()
            } operation: {
                // This check is necessary in case this code runs after the task was
                // cancelled. In which case we want to bail right away.
                try Task.checkCancellation()
                
                return try await withUnsafeThrowingContinuation { continuation in
                    // This check is necessary in case this code runs after the task was
                    // cancelled. In which case we want to bail right away.
                    guard !Task.isCancelled else {
                        continuation.resume(throwing: Task.CancellationError())
                        return
                    }
                    
                    cancellableWrapper.cancellable =
                    handleEvents(receiveCancel: {
                        // We don't get a cancel error when cancelling a publisher, so we need
                        // to handle if the publisher was cancelled from the
                        // `withTaskCancellationHandler` here.
                        continuation.resume(throwing: Task.CancellationError())
                    }).sink { completion in
                        if case let .failure(error) = completion {
                            continuation.resume(throwing: error)
                        } else if !didSendValue {
                            continuation.resume(throwing: AsyncError.valueWasNotEmittedBeforeCompletion)
                        }
                    } receiveValue: { value in
                        continuation.resume(with: .success(value))
                        didSendValue = true
                    }
                }
            }
        }
    }
}

Also, the ability to turn a Publisher into an AsyncSequence or AsyncThrowingSequence would be very helpful as well.

3 Likes
115

Classes with mutable state are not safe to send across concurrency domains, correct?
To send something across it should conform to Sendable, correct?
If the class Foo conforms to Sendable the compiler will emit errors about Foo.value being mutable.
If the class however doesn't conform, no errors are emitted and the code below runs just fine.
But it's actually not safe from data races, is it?

I expected an error as well if Foo doesn't conform to Sendable and if it's used across concurrency domains. Or was I wrong to expect this?

Using xcode beta 2.

@main
struct MyApp
{
    static func main() async
    {
        let foo = Foo(value: "foo")
        let fooActor = FooActor()
        let barActor = BarActor()
        await barActor.use(foo: foo)
        await fooActor.use(foo: foo)
        await barActor.send(foo: foo, to: fooActor)
    }
}

final class Foo //: Sendable
{
    var value: String 
    
    init(value: String) 
    {
        self.value = value 
    }
}

actor FooActor
{
    func use(foo: Foo)
    {
        print("foo actor using: \(foo.value)")
    }
}

actor BarActor
{
    func send(foo: Foo, to target: FooActor) async
    {
        foo.value = "visited bar"
        await target.use(foo: foo)
    }
    
    func use(foo: Foo)
    {
        print("bar actor using: \(foo.value)")
    }
}