cachedServices is not thread safe

mycroftcanner commented 5 years ago

Any chance you could make the application cache thread safe?

In general I only let the strict necessary on the main thread and I use Texture so everything runs in the background except for strict UIKit and some SwiftUI stuff

Maybe implementing a lock on the cachedServices would help:

            cachedServices[registration.cacheKey]

hmlongco commented 5 years ago

You mean like using mutex to do thread locking?

    public final func resolve<Service>(resolver: Resolver, registration: ResolverRegistration<Service>, args: Any?) -> Service? {
        pthread_mutex_lock(&mutex)
        defer { pthread_mutex_unlock(&mutex) }
        if let service = cachedServices[registration.cacheKey] as? Service {
            return service
        }
        let service = registration.resolve(resolver: resolver, args: args)
        if let service = service{
            cachedServices[registration.cacheKey] = service
        }
        return service
    }

hmlongco commented 5 years ago

I might mention that if you're using Resolver in a project it might be worth your while to spend the 15-20 minutes it would take to read the documentation... ;)

Further, Resolver: Is tested in production code. Is thread safe (assuming your objects are thread safe). Has a complete set of unit tests. Is well-documented.

mycroftcanner commented 5 years ago

Sorry about that... I am so used to project not using mutex or barriers...

I just assumed or was tired as it was right in front of my eyes.

mycroftcanner commented 5 years ago

I had a bad access on cachedServices[registration.cacheKey].

I didn't investigate and just assumed. A quick fix was to call the registration early.

Now I can't reproduce the problem. What do you think could have caused a bad access on cachedServices?

mycroftcanner commented 5 years ago

Finally managed to reproduce the issue:

@hmlongco any ideas what would be the cause?

mycroftcanner commented 5 years ago

The factory instance is injected in my SceneDelegate:

class SceneDelegate: UIResponder, UIWindowSceneDelegate {
    var window: UIWindow?

    @Injected private var auth: Auth
    @Injected private var factory: WindowFactory

    [...]

    func scene(_ scene: UIScene, willConnectTo session: UISceneSession, options connectionOptions: UIScene.ConnectionOptions) {
        if let windowScene = scene as? UIWindowScene {
            window = factory.makeWindow(windowScene: windowScene)
        }
    }

then my environment objects are injected inside the Factory class:

    // Used a EnvironmentObjects in SwiftUI view hierarchies
    @Injected private var auth: Auth
    @Injected private var settings: SettingsStore

Is there a race condition or am I doing something wrong?