Is this an ok solution to achieve shared instance of actor using async init?

Sajjon · March 4, 2023, 6:41pm

I compile an SPM target with:

.unsafeFlags([
    "-Xfrontend", "-warn-concurrency",
    "-Xfrontend", "-enable-actor-data-race-checks",
]),

Conceptually, I wanna do this, but can't, due to async' call cannot occur in a global variable . So I need to find some alternative:

public final actor FooStore {

    public static let shared: FooStore = await FooStore() // ❌ 'async' call cannot occur in a global variable initializer

    private let foo: Foo
    private init() async {
        self.foo = await Self.initFooUsingMustBeAsyncMethod()
    }
}

So instead I naively try this:

public final actor FooStore {
    private static var _shared: FooStore?

    /// Completely fine that this is a function
    public static func shared() async -> FooStore {
        // ⚠️ WARNING: Reference to static property '_shared' is not concurrency-safe because it involves shared mutable state
        if let _shared {
            return _shared
        }
        let shared = await FooStore()
        
        // ⚠️ WARNING: Reference to static property '_shared' is not concurrency-safe because it involves shared mutable state
        _shared = shared
        return shared
    }

    private let foo: Foo
    private init() async {
        self.foo = await Self.initFooUsingMustBeAsyncMethod()
    }
}

OK I understand the warning and take it seriously, bad idea!

So I figured I'd try fixing those warnings, by... wrapping an actor around it..?! And hey, it compiles without warning!

public final actor FooStore {
    
    private final actor StoreOfFooStore {
        private var _shared: FooStore?
        fileprivate func shared() async -> FooStore {
            if let shared = _shared {
                return shared
            }
            let shared = await FooStore()
            _shared = shared
            return shared
        }
    }
    private static let storeOfSharedFooStore = StoreOfFooStore()

    /// Completely fine that this is a function
    public static func shared() async -> FooStore {
        await Self.storeOfSharedFooStore.shared()
    }
    
    private let foo: Foo
    private init() async {
        self.foo = await Self.initFooUsingMustBeAsyncMethod()
    }
}

Is this an OK solution, as in thread safe, data race safe?

ole · March 8, 2023, 6:06pm

I think you have a potential reentrancy problem in StoreOfFooStore.shared(). Since let shared = await FooStore() may suspend, another task could call it again before the _shared property becomes non-nil. This means you'd run FooStore.init multiple times.

Sajjon · March 9, 2023, 8:24am

Thank you @ole ! Howcome Swift compiler did not find flag this as a warning? Did I manage to fool it (and myself...) by use of the nested actor (too complex for static analysis?)

And I guess this version has the same problem, since it also contains the line: let shared = await FooStore()

public final actor FooStore {

	/// ActorIsolated by PF: https://github.com/pointfreeco/swift-dependencies/blob/main/Sources/Dependencies/ConcurrencySupport/ActorIsolated.swift#L42
	private static let _shared = ActorIsolated<FooStore?>(nil)

	public static var shared: FooStore {
		get async {
			if let shared = await _shared.value {
				return shared
			}
			let shared = await FooStore()
			await _shared.setValue(shared)
			return shared
		}
	}

	private let foo: Foo
	init() async {
        self.foo = await Self.initFooUsingMustBeAsyncMethod()
	}
}

Is it (actor with static shared using async init of Foo?) at all achievable?

I guess I would be open to some kind of solution using DispatchSemaphore or NSRecursiveLock or something? Or does anyone have a better idea?

Sajjon · March 9, 2023, 10:31am

Can I used ManagedCriticialState from AsyncAlgorhtms perhaps?

Alejandro_Martinez · March 9, 2023, 7:34pm

Just to touch on this, current actors in swift are reentrant so for the language this code is totally valid. It’s up to the programmer to maintain invariants between suspension points, something to be aware. so nothing to do with the code you have going here ^^

ole · March 10, 2023, 3:32pm

As @Alejandro_Martinez said, Swift Concurrency won't protect you from "logic races", i.e. code that is totally safe, but doesn't do the right thing. Concurrency is still hard! You have to take into account that the entire non-local state of your program may have changed after a suspension point.

Sajjon · March 10, 2023, 3:52pm

Sure. But I’m still asking for help/guidance on how to achieve what I want.

Which is an (safe/correct) actor that has a shared instances, which needs the be initialized asynchronously.

Surely I cannot be the only person in the world wanting to achive this use case?

Sajjon · September 13, 2023, 8:13pm

Update: I asked @lorentey on Mastodon if ManagedAtomicLazyReference could help here, and he said it could - indeed it did! ManagedAtomicLazyReference is marvellous in this use case.

(The code below is also in this Swift Package (just a testTarget not a lib))

I managed to prove @ole and @Alejandro_Martinez concerns - thank you very very much btw - that my naive implementation indeed has reentrancy problems - run the test test_reentrant like 10 times, it will fail. But I've run test_managed many many times now (a couple of hundred thousand) and it never fails.

Anyone should be able to copy paste this into a test in a SPM package with swift atomics as dependency to play around with, hope it helps someone.

import XCTest
import Atomics // using version 1.1.0
import Foundation

// MARK: Profile

/// Some shared application data akin to a user, which we only ever want one of, typically saved in keychain.
public struct Profile {
	public typealias ID = UUID
	public let id: ID
	public init(id: ID = .init()) {
		self.id = id
	}
}

// MARK: Keychain mock
actor KeychainMock: GlobalActor {
	var profile: Profile?
	static let shared = KeychainMock()
	func setProfile(profile: Profile) async {
		self.profile = profile
	}
}

struct KeychainProfileProvider {
	typealias Value = Profile
	static func provide() async -> Profile {
		if let profile = await KeychainMock.shared.profile {
			return profile
		}
		let new = Profile()
		await KeychainMock.shared.setProfile(profile: new)
		return new
	}
}

// MARK: ProfileStoreProtocol
public protocol ProfileStoreProtocol {
	static func shared() async -> Self
	func getProfile() async -> Profile
	func setProfile(_ profile: Profile) async
}

// MARK: ManagedAtomicProfileStore

/// A ProfileStore which uses `ManagedAtomicLazyReference` under the hood.
public final actor ManagedAtomicProfileStore: ProfileStoreProtocol {
	private static let managedAtomicLazyRef = ManagedAtomicLazyReference<ManagedAtomicProfileStore>()
	private var profile: Profile
	private init() async {
		self.profile = await KeychainProfileProvider.provide()
	}
}

// MARK: ManagedAtomicProfileStore + ProfileStoreProtocol
extension ManagedAtomicProfileStore {

	public static func shared() async -> ManagedAtomicProfileStore {
		await managedAtomicLazyRef.storeIfNilThenLoad(ManagedAtomicProfileStore())
	}

	public func getProfile() async -> Profile {
		profile
	}
	
	public func setProfile(_ profile: Profile) async {
		self.profile = profile
	}
}


// MARK: ReentrantProfileStore (Dont use)

/// An implementation with reentrancy problems, as shown below.
public final actor ReentrantProfileStore: ProfileStoreProtocol {
	
	private static let metaStore = MetaStore()
	
	private var profile: Profile
	private init() async {
		self.profile = await KeychainProfileProvider.provide()
	}
}

extension ReentrantProfileStore {
	private final actor MetaStore {
		private var _shared: ReentrantProfileStore?
		fileprivate func shared() async -> ReentrantProfileStore {
			if let shared = _shared {
				return shared
			}
			let shared = await ReentrantProfileStore()
			_shared = shared
			return shared
		}
	}
}

// MARK: ReentrantProfileStore + ProfileStoreProtocol
extension ReentrantProfileStore {

	public static func shared() async -> ReentrantProfileStore {
		await Self.metaStore.shared()
	}
	
	public func getProfile() async -> Profile {
		profile
	}
	
	public func setProfile(_ profile: Profile) async {
		self.profile = profile
	}
}



// MARK: Test Helper

/// This test method has not been implemented with any particular thoughts in mind
/// mostly creating a bunch of unstructured task and reading/setting profile on shared
/// profile store interleaved with some `Task.yield()`'s which seem to what makes
/// the ReentrantProfileStore fail.
func doTestProfileStore<ProfileStore: ProfileStoreProtocol>(
	type: ProfileStore.Type = ProfileStore.self
) async {
	let t0 = Task {
		await ProfileStore.shared()
	}
	await Task.yield()
	var profile = await ProfileStore.shared().getProfile()
	await Task.yield()
	let t1 = Task {
		await ProfileStore.shared()
	}
	let t2 = Task {
		await ProfileStore.shared()
	}
	await Task.yield()
	profile = await ProfileStore.shared().getProfile()
	await Task.yield()
	let t3 = Task {
		await ProfileStore.shared()
	}
	await Task.yield()
	await ProfileStore.shared().setProfile(profile)
	await Task.yield()
	let t4 = Task {
		await ProfileStore.shared()
	}
	let t5 = Task {
		await ProfileStore.shared()
	}
	await Task.yield()
	profile = await ProfileStore.shared().getProfile()
	await Task.yield()
	let t6 = Task {
		await ProfileStore.shared()
	}
	let t7 = Task {
		await ProfileStore.shared()
	}
	await Task.yield()
	await ProfileStore.shared().setProfile(profile)
	await Task.yield()
	let t8 = Task {
		await ProfileStore.shared()
	}
	let t9 = Task {
		await ProfileStore.shared()
	}
	
	let tasks = [t0, t1, t2, t3, t4, t5, t6, t7, t8, t9]
	var values = Set<Profile.ID>()
	for task in tasks {
		let profile = await task.value.getProfile()
		values.insert(profile.id)
	}
	XCTAssertEqual(values.count, 1) // will fail for `test_reentrant` sometimes
}

// MARK: Test
final class ProfileStoreTests: XCTestCase {
	
	func test_managed() async {
		await doTestProfileStore(type: ManagedAtomicProfileStore.self)
	}
	
	
	// Fails ~10% of runs
	func test_reentrant() async {
		await doTestProfileStore(type: ReentrantProfileStore.self)
	}

}