AliSoftware / Dip

Simple Swift Dependency container. Use protocols to resolve your dependencies and avoid singletons / sharedInstances!
MIT License
978 stars 75 forks source link

Singleton objects created multiple times #160

Closed antonselyanin closed 7 years ago

antonselyanin commented 7 years ago

Objects that are registered as singletons could be created multiple times during resolving circular dependencies. I'll make a pull request with demo tests shortly.

This issue might be connected with #159

antonselyanin commented 7 years ago

I could try to fix it ) I actually wanted to fix it before reporting but noticed issue #159

ilyapuchka commented 7 years ago

Hi @antonselyanin This is an expected behaviour described in the docs - https://github.com/AliSoftware/Dip/wiki/circular-dependencies. In your example when resolving Client when its constructor is called container will first need to resolve Server. After server is resolved container will resolve its properties which will in turn try to resolve Client again. But the client constructor did not return yet and thus is not stored in resolved instances collection. This will cause second instance of client to be created. Server was already resolved (its constructor returned) so its reference is stored in resolved instances, so it will be reused while resolving client at this moment. After its done first instance of client will be released and second will be returned. For the same reason when using unique causes stack overflow. The same will happen when using constructor injection for both dependencies. That's the common problem of DI containers trying to solve circular dependencies and it is usually advised to use property injection for both of dependencies or to refactor code to avoid circular dependencies per se. You can also ensure single instances using singleton pattern (singleton scope is not the same as it only ensures that there will be only single instance of this type returned by container)

antonselyanin commented 7 years ago

This makes sense, you are right. Actually we experienced this first time with Swinject several months ago. For some reason, I thought they fixed it. But, I guess, you can't "fix" it without violating the contract that you return a completely initialized object from a 'resolve' method.

Thanks for clarifying it!

About unique - in Swinject they detect that your resolution path is too deep, so they assume it's an invalid circular depedency and just produce a fatal error. This helped us to detect an error in our dependency graph a couple of times. IMO, a nice diagnostics, but not that necessary.

    private var maxResolutionDepth: Int { return 200 }

    private func incrementResolutionDepth() {
        guard resolutionDepth < maxResolutionDepth else {
            fatalError("Infinite recursive call for circular dependency has been detected. " +
                "To avoid the infinite call, 'initCompleted' handler should be used to inject circular dependency.")
        }
        resolutionDepth += 1
    }