Support for Async/Await Concurrency in Factory

[TheCoordinator]

Hey @hmlongco,

First of all would like to say well done on creating both Resolver and Factory. I have been using Resolver in production for over a year now and really happy on how much it helped us clean up our code in regards to dependency injection.

A pain point that we had with Resolver was when we migrated our code to the the new Concurrency model when using Mainactor or other types of Actors in general.

Now that Factory seems to be a full re-write of Resolver with new features in mind, I wonder if now is a good time for you (us?) to think a bit more on how to natively support concurrency out of the box. Looking at the code, the actor model should be able to help you a lot when it comes to thread safety.

Is this something that you're open to implement/discuss for Factory?

[kamaldeep-earnin]

I know this is for the author but just budging in since I also looked into this for my current project. Locks in general are more performant than actors. Resolving dependencies should be fast.

[TheCoordinator]

Interesting, haven't personally done any benchmarking for this so it'd be good to see the comparison for them if you have any references?

Thinking if semantically would make any difference if we have Concurrency fully baked in, i.e. if we're injecting a Service into a ViewModel that needs to be of @MainActor.

[kamaldeep-earnin]

I haven't done any benchmarking myself but did some finding out to see if someone has already done it. Heres one post that benchmarks the performance of concurrency approaches in a controlled environment and with similar constructs. Instead of NSRecursiveLock this uses os_unfair_lock_s which is kind of similar in behavior.

Also, I do agree having actor support in this library as an option would be a great addition but that should be wrapped in SDK API availability since deployment target for this lib is iOS 11.

[hmlongco]

I haven't used Swift Concurrency enough to be aware of all of the pain points. Most of what I've done thus far is marking certain functions MainActor as opposed to marking the entire class as such. (Or using pure actors.)

In particular I'm not certain of the ramifications of doing this and how such support would cascade throughout the entire system (especially in regard to scopes).

[hmlongco]

Insofar as I can tell there's one use case that's problematic and that's when an entire class is marked with MainActor.

extension Container {
    static var myActor = Factory { SomeActor() }
    static var mainActorTest = Factory { MainActorFuncTest() }
//    static var mainActorTest2 = Factory { MainActorTest() } // fails
}

class MainActorFuncTest {
    @MainActor
    func load() async -> String {
        return "Acting"
    }
}

@MainActor
class MainActorTest {
    func load() async -> String {
        return "Acting"
    }
}

actor SomeActor {
    func load() async -> String {
        return "Acting"
    }
}

class SomeActorParent {

    @Injected(Container.myActor) var myActor
    let myTest = Container.mainActorTest()

    @MainActor
    func test() async {
        let result = await myActor.load()
        print(result)
    }

}

As to using it internally, I don't think I can. A class can request a dependency which in turns requests a dependency which in turns requests a dependency, and so on. That's why most of the locks used are recursive.

[TheCoordinator]

@hmlongco the MainActorTest I believe is a good usecase, we for example use that quite a lot to create our ViewModels which indeed need to be marked as @MainActor especially if being used inside VCs or SwiftUI views to make sure async code doesn't mutate the VM state from a background thread.

Any workaround we can do for this?

Vaguely remember we could achieve this via Sendable closures from last year WWDC talks.

[TheCoordinator]

Agree, internally may not work especially if this is supposed to be used for pre iOS 13 usage. However dedicated actors are made exactly for the purpose of what you're using so no matter what thread the request is coming in from it always protects the state. This however is only true if using it within the new Concurrency model and won't have any effect if being called directly from a background queue such as GCD.

Maybe there's a workaround we can use to accommodate both cases?

[hmlongco]

I believe is a good usecase, we for example use that quite a lot to create our ViewModels which indeed need to be marked as @MainActor especially if being used inside VCs or SwiftUI views to make sure async code doesn't mutate the VM state from a background thread.

From my perspective that's better off isolated to the actual functions that we know have asynchronous behavior.

class MainActorFuncTest {
    @MainActor
    func load() async -> String {
        return "Acting"
    }
}

As you've no doubt noticed, once you added MainActor to the entire class you begin to find you need to add them everywhere and so the little suckers start propagating throughout your code.

Pretty soon you start adding then just to silence the warnings... and not many people realize or understand what little helpers Swift is now adding to all of that code to make that MainActor attribute do its thing.

[hmlongco]

Rewatched a few WWDC videos and realized that you can do the following...

extension Container {
    static var mainActorTest2 = Factory { MainActorTest() }
}

@MainActor
class MainActorTest {
    let text: String
    nonisolated init() {
        text = "Acting"
    }
    func load() async -> String {
        return text
    }
}

Note how the init function is marked as nonisolated. That allows it to be constructed in the factory even if the entire class is marked as @MainActor.

[rsaccone]

I am not sure I understand what the issue is with Swift Concurrency and MainActor. Main actor says that the method must run on the main actor (thread) and the Swift runtime will context switch into the main actor (thread) when a method that is marked with MainActor is invoked. Now if a method running on the main actor goes off and makes a call into Factory, that thread will continue to be used. I am guessing folks are worried about the use of the NSRecursiveLock inside of Factory if they are used on the main actor (thread)? Based on what I’ve seen in a WWDC21 session (and some experience with using NSLock with Swift Concurrency) this shouldn’t be an issue because:

“With Swift concurrency primitives like await, actors, and task groups, these dependencies are made known at compile time. Therefore, the Swift compiler enforces this and helps you preserve the runtime contract. Primitives like os_unfair_locks and NSLocks are also safe but caution is required when using them. Using a lock in synchronous code is safe when used for data synchronization around a tight, well-known critical section. This is because the thread holding the lock is always able to make forward progress towards releasing the lock. As such, while the primitive may block a thread for a short period of time under contention, it does not violate the runtime contract of forward progress."

https://developer.apple.com/videos/play/wwdc2021-10254/?time=1493

From the looks of things I believe the usage of locks in Factory align with that statement.

[hmlongco]

I think the original discussion was based more around using actors internally as opposed to locks. Thing is, when you start using actors you can guarantee proper locking behavior, but every time you use await you have the potential for the system to switch threads... which could have ramifications for the caller attempting to resolve a given service.

[TheCoordinator]

Yes @hmlongco is spot on, the main intention of this discussion was to see if we can use actors instead of NSLock and natively allow consumers safely access factory. Believe getting a dependency from an async call is a rare case however if we do support that natively it'll be possible. What I'm not sure is what the implications will be for users who have not adopted Concurrency yet.

[TheCoordinator]

Also think if we use MainActor Swift guarantees that all other MainActor calls will be running on the same thread without switching which is unlike DispatchQueues.

[hmlongco]

The more it's discussed the more I'm sure that you can't do it and maintain the existing contract for synchronous dependency resolution. This is especially problematic for the property wrappers like @injected which must complete prior to object initialization.

[rsaccone]

Thanks I get the discussion now. Yes @hmlongco I agree there’s no good way to support synchronous contract for dependencies and make the switch to actors. A few weeks ago I ran into a similar issue where I was starting from a context that had to remain synchronous and wanted to use an actor under the covers. By the time I pieced it together it the end result felt overly complex because it was jumping from synchronous to asynchronous and back to synchronous. I also didn’t like how long it would have booked the thread with the thread the synchronous part was running on.

[TheCoordinator]

OK, I've done a PoC now and starting to agree with you both! Seeing a lot of strange behaviour specifically with PropertyWrappers, to be honest other than that it's quite solid.

[TheCoordinator]

An update on this, I went ahead and implemented Factory instead of Resolver in one of my projects which is fully on new Concurrency and can confirm that I had no particular issues with the migration. So @hmlongco feel free to close this or keep it open as a reference. All good on my side.