Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Singleton objects created multiple times #160

Closed
antonselyanin opened this issue May 20, 2017 · 3 comments
Closed

Singleton objects created multiple times #160

antonselyanin opened this issue May 20, 2017 · 3 comments

Comments

@antonselyanin
Copy link

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
Copy link
Author

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

@ilyapuchka
Copy link
Collaborator

ilyapuchka commented May 20, 2017

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
Copy link
Author

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
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants