Making actor non reentrant

What’s the easiest way to prohibit my actor’s methods being called reentrantly?

actor A {
    func foo() async { … }
}
let a = A()
Task {
    await a.foo() // takes 10 seconds
}
// after a second
Task {
    await a.foo() // want this to await until the first foo is finished, and then do the actual call
}
1 Like

Not using actors (or asynchronous functions)?

4 Likes

There is no way, but the recommendation is to put code that may break preconditions across awaits in a single sync function in the actor isolation context. That way you think about the logic transactionally and avoid any reentrancy problems.
Not the panacea but helps to think this way.

2 Likes

The easiest pointed by @ktraunmueller, but it works if you can make it so. If not, you have to manually serialize calls (using continuations more likely). There were topics with code examples. In simples case you just need a flag that guards and array of continuations.

1 Like

A possible way is to setup a stream that enqueues calls to foo, as below:

actor MyActor {
    private struct QueueRequest {
        var continuation: CheckedContinuation<Void, Never>
        var job: () async -> Void
    }

    private let queueContinuation: AsyncStream<QueueRequest>.Continuation!
    private let queueTask: Task<Void, Never>

    init() {
        // Prepare the queue of foo() invocations
        let (stream, queueContinuation) = AsyncStream.makeStream(of: QueueRequest.self)
        self.queueContinuation = queueContinuation
        self.queueTask = Task {
            for await request in stream {
                await request.job()
                request.continuation.resume()
            }
        }
    }

    deinit {
        queueTask.cancel()
    }

    func foo() async {
        await withCheckedContinuation { continuation in
            let request = QueueRequest(continuation: continuation) {
                await self.nonReentrantFoo()
            }
            queueContinuation.yield(request)
        }
    }

    // This function is not reentrant
    private func nonReentrantFoo() async {
        print("Start foo")
        try? await Task.sleep(for: .seconds(1))
        print("End foo")
    }
}
Quick check
final class MyActor Tests: XCTestCase {
    func testFooIsNotReentrant() async throws {
        let a = MyActor()

        // Prints:
        // - Start foo
        // - End foo
        // - Start foo
        // - End foo
        await withTaskGroup(of: Void.self) { group in
            group.addTask {
                await a.foo()
            }
            group.addTask {
                await a.foo()
            }
        }
    }
}

You can also use a Semaphore (there are several ones online, including mine), but take care of priority inversion:

import Semaphore

actor MyActor {
    private let fooSemaphore = AsyncSemaphore(value: 1)

    func foo() async {
        await fooSemaphore.wait()
        defer { fooSemaphore.signal() }

        // The semaphore makes sure the code below
        // is never called in a reentrant way.
        print("Start foo")
        try? await Task.sleep(for: .seconds(1))
        print("End foo")
    }
}

And see also ConcurrencyRecipes/Recipes/Structured.md at main · mattmassicotte/ConcurrencyRecipes · GitHub

8 Likes

There's an old-school way of doing it using semaphores but your function foo() won't be async (and actor will become a class) as Swift won't allow it. So:

class A {
	func foo() {
		semaphore.wait()
		// ...
		semaphore.signal()
	}

	private let semaphore = DispatchSemaphore(value: 1)
}

You have to be careful with it of course and call foo() from within Task { } to ensure you don't block your app's main paths.

Edit: A can still be an actor if you need it, then foo() will have to be marked nonisolated.

1 Like

You really shouldn't do this: mixing-up GCD semaphores is a bad idea, even outside of actors and async functions. There were several detailed discussions on the topic:

But I specifically said A shouldn't be an actor, or otherwise foo() should be nonisolated, otherwise Swift warns you about the usage of wait() within async context.

Semaphores are fine if you know what you are doing. They are almost as old as computing.

Huh, had to dig the thread a bit, but:

(link deserves it's own extraction, I think: Xcode 10 Release Notes: Static Analyzer)

Look @vns, I know from our previous interactions that you won't be convinced no matter what I say, but yes, old-school synchronization is dangerous but sometimes it simplifies your code greatly if you know what you are doing. If you don't, then don't.

1 Like

I'm pretty sure developers of Vision framework did know what they were doing, yet it now allows to have a deadlock in Swift Concurrency.

Anyway, I am not trying to convince anyone there, but highlight (firstly, to author), that this is not a good idea.

As far as I can tell they used semaphores in pretty complex code and in a non-trivial situation, so I'm not surprised it came out problematic.

1 Like

Looking at the original problem, which looks simple from the outset, to me it feels like people are trying to make everything an actor, and nobody asks the question, is all of this trouble really worth it?

I know this position is a bit heretic, but I for myself have decided to not use actors, because it a) forces me to alter my designs in ways I don't like, and b) it makes even the simplest problems much more difficult (e.g. via actor reentrancy).

3 Likes

I personally find Swift's concurrency system uncontrollable and unobservable, which makes it hard to use the system correctly.

Given that still in its infancy, I hope things will improve one day.

2 Likes

i find that traditional object-oriented programming patterns don’t work well with actors, but i’ve had more success using functional programming patterns, which is funny given that actors are objects.

basically the idea is each async function forms a concurrency domain, and the various concurrency domains synchronize with each other through the actor’s properties and AsyncStreams.

here’s an untyped sketch to illustrate:

actor A
{
    var sharedState
}
extension A 
{
    func f() async
    {
        var fLocalState
        for await ... in fStream
        {
            ...
        }
    }
    func g() async
    {
        var gLocalState
        for await ... in gStream
        {
            ...
        }
    }
}
4 Likes

(Though going slightly off-topic but) I'm curious, why do you think it's uncontrollable and unobservable?

I view structured concurrency in general as one of the greatest advances in programming languages in recent couple of decades, though also of course like any software abstraction it is susceptible to misunderstanding and abuse. And Swift's implementation of it in particular seems pretty good to me.

Modern mobile devices can have 4-6 CPU cores and with older approaches you'd use only 1 or 1.5 cores at best, the rest would just sit idly. You do need some safe mechanism of using more CPU cores and structured concurrency gives you a pretty elegant tool with guarantees that you won't hit your finger instead of the nail.

I'm generally fine with the direction Swift is moving in, so what is wrong with it?

3 Likes

To me the major hurdle of async/await/actors is that the code "looks" too linear / synchronous while in fact it is nothing but:

func foo() async {
	checks
	await foo()
	await bar()
}

Every await is a suspension point, after which everything could be different e.g. the checks you did before await are potentially no longer valid and if you still need to ensure they are held you have to repeat them again. Very easy to make mistakes in anything but trivial examples and compiler or runtime can't help you anymore (before you'd at least were getting low level data races on a good day – those crashes forced you to pause and think carefully about what you are doing, now with actors not even that). We got rid of callback hell to only get ourselves into a different place – suspension point hell.

4 Likes

More like the illusion of it, in my opinion. Most developers don't measure for performance and happily hop threads just to spend a few milliseconds for e.g. accessing a dictionary or calling other async functions. Which probably doesn't even offset the cost of the concurrency runtime itself (or the fact that each cpu frequency is throttled when more cpus are running, which you need to offset as well). Doing actual parallelism is a whole different topic, you need to carefully prepare enough work, and slice it appropriately (with no dependency between each piece) in order to fill up the cpus efficiently, and always measure along the way. Furthermore I'm not sure you would want to starve a limited pool that is shared for all other work.

3 Likes

In many ways the above code is equivalent to (intentionally verbose):

func foo(completion: () -> Void) {
    checks()
    foo(completion: {
        bar(completion: {
            completion()
        })
    })
}

except the async/await version may use your CPU cores more efficiently (and ensure you don't screw up access to shared data) while the callback version will execute on a single thread unless you explicitly make it do otherwise.

But the problem of higher-level data consistency is still there. If you don't understand callbacks then neither will you "get" async/await. In which case you should probably switch to a simpler programming language and simpler problems as well :slight_smile:

While I agree with the statement in more broad sense, as once you move towards actual performance gains, you have to be careful. Still, for many use cases that is greatly simplifies significant number of tasks, and reduces programmer error. I mean, many were in illusion anyway, but at least for now there are fewer ways to make a mistake.

Kinda ironically that one of the major benefits of structured concurrency turns out the major hurdle, heh. The approach is definitely not easy to switch to in a snap, as it requires to change how you think and reason, so that takes some time, but once you get used to, structured approach bears many benefits. It’s just less usual.

It somewhat forces you to structure differently the code, so you won’t run into that issues. I find that most of the time it highlights problems, that were hidden otherwise.