Lazy InstanceWrapper can't provide thread safety

[danya61]

Yesterday I faced some crash in 302:Container.swift:

        guard let currentObjectGraph = currentObjectGraph else {
            fatalError("If accessing container from multiple threads, make sure to use a synchronized resolver.")
        }

In my case, I lazily resolve dependencies, and judging by the stack trace, Lazy<Service> appeared in all the crashes.
After a while, I seem to have found the problem.

Problem

Imagine we register some dependency with a Lazy<Service> argument in init:
container.register(Person.self) { r in PetOwner(pet: r.resolve(Lazy<Animal>.self)!) }

Sometime later we resolve Person instance by using SynchronizedResolver under the hood which is looks absolutely thread safe:
let person = container.synchronize().resolve(Person.self)!

After a while, we decide to use the instance:
let name = pet.instance.name

The stacktrace shows that the next step is to fall into the resolveAsWrapper method's closure factory:

if let entry = getEntry(for: key) {
       let factory = { [weak self] in self?.resolve(entry: entry, invoker: invoker) as Any? }
       return wrapper.init(inContainer: self, withInstanceFactory: factory) as? Wrapper
} else { /**/ }

The problem is that
self?.resolve(entry: entry, invoker: invoker) as Any?
Never executed in a thread-safety way (self.resolve is not SynchronizedResolver method)
It seems to me that because of this, we can get a crash when trying to resolve a dependency from different threads at the same time (And this is probably the reason for my series of crashes).
Please correct me if my thoughts are not correct.

Possible solution

Rewrite a part of resolveAsWrapper method:

        if let entry = getEntry(for: key) {
            let factory = { [weak self] in
                self?.lock.sync { // provide thread-safety
                    self?.resolve(entry: entry, invoker: invoker) as Any?
                }
            }
            
            return wrapper.init(inContainer: self, withInstanceFactory: factory) as? Wrapper
        } else {
            return wrapper.init(inContainer: self, withInstanceFactory: nil) as? Wrapper
        }

I don't know how correct this is within such a large library, and how much it will affect the overall performance, but it should fix the multithreaded environment issue.

[welshm]

This is still an issue. Our workaround currently is to resolve everything on the same thread, which is less than ideal.

Would it be possible to change the SynchronizedResolver from a wrapper class to a property of the Container ? Then one could instantiate or set that property, so that when the Wrapper resolves it could still enforce using the locks.

[welshm]

I think this is fixed in the latest release if you want to try @danya61 @chosa91

[danya61]

@welshm thanks for the solution, seems like it works correct