Swinject: Problem with postponed `initCompleted` calls

[petalvlad]

In the 1.1.4, calls of initCompleted are postponed (see implementation https://github.com/Swinject/Swinject/pull/134/commits/35011957dad63cf11c0e06c4b7ab3d4a91bc9ef2#diff-e549a00f41e3fe6ad1e35a13ebcaddb0L202).

Let's assume that we have A which has dependency B, B which has dependency C and C which has dependency B.

A<-B<->C

container.register(A.self) { r in
    return A(b: r.resolve(B.self)!)
}

container.register(B.self) { r in
    return B()
}.initCompleted { (r, b) in
    b.c = r.resolve(C.self)!
}

container.register(C.self) { r in
    return C(b: r.resolve(B.self)!)
}

If we want to call method foo of B inside A's initializer, we'll get an error, because dependency C of B is not initialized yet.

class A {
    init(b: B) {
        b.foo()
    }
}

class B {
    var c: C! {
        didSet {
            print("C has been set")
        }
    }

    func foo() {
        print(c)
    }
}

class C {
    let b: B
    init(b: B) {
        self.b = b
    }
}

Above code was working in 1.1.2, because initCompleted was called at the end of resolving an instance.

Is this behavior expected in 1.1.4 or we can consider it as a bug?

[yoichitgy]

Thanks @petalvlad for submitting the issue! This is not an expected behavior. Let's find a nice way to satisfy both the #134 and your cases. Do you have any idea?

@marcorei, I also would like to hear your idea!

[marcorei]

@petalvlad @yoichitgy I am sorry the PR broke this case, although I am not sure I would consider this a bug.

Because of the property being an optional and the method being named initComplete I must admit that I thought one would not expect these objects to be available until after the entire non-circular part of the tree was initiated.

I think it is not possible to resolve a graph with circular dependencies without either duplicating objects or postpone their injection.

A possible solution could be to call initCompleted directly (instead of delaying the call) if the ObjectScope is graph since the promise that "instances resolved in factory closures are shared during the resolution of the root instance to construct the object graph" kind of excludes the initComplete closures already.

I think this could be viable since one does not explicitly request a Singleton when using the default ObjectScope. That would not solve this issue for container and hierarchy but could perhaps be a good compromise.

[marcorei]

Another thought: relying on circular dependencies during initialization has only worked in some cases before the patch.

If we modify the example above and only use two dependencies A <-> B, you should also get an error without postponing the initComplete call.

container.register(A.self) { r in
    return A(b: r.resolve(B.self)!)
}

container.register(B.self) { r in
    return B()
}.initCompleted { (r, b) in
    b.a = r.resolve(A.self)!
}

class A {
    init(b: B) {
        b.foo()
    }
}

class B {
    var a: A! {
        didSet {
            print("A has been set")
        }
    }

    func foo() {
        print(a)
    }
}

[yoichitgy]

Thanks @marcorei for giving us your idea😃 It's helpful for me to investigate the issue. The problem is not your fault. I merged the PR before I check it carefully. I agree that resolving circular dependency is a difficult problem for a container without reflection.

A possible solution could be to call initCompleted directly (instead of delaying the call) if the ObjectScope is graph since the promise that "instances resolved in factory closures are shared during the resolution of the root instance to construct the object graph" kind of excludes the initComplete closures already.

We can take this as a quick patch. I'll also investigate a possibility to solve the case #133 without pending the calls of initCompleted.

[marcorei]

That's kind of you to say, @yoichitgy ! And you're right, calling it impossible was hastly 😓

I'm looking at Dagger 2 (Java), which does not use reflection and supports exactly these cases (peta's case would work, mine wouldn't). From the top of my head I'd say this might as well work by registering an empty "placeholder" to a pool around this line and using these to satisfy dependencies during the non-delayed initComplete calls. The placeholder would be filled after the initialization is completed.
This placeholder could be similar to an Optional (but no value type), similar to Dagger's Lazy.

I have to give it some more thought next week!

[yoichitgy]

@marcorei, thank you very much for your investigation with Dagger👍 I'll also check it and how the problem can be fixed, but your investigation and idea is very appreciated!!! Waiting the next week.

[jakubvano]

Resolved by #164

[yoichitgy]

@petalvlad, v1.1.5 fixed the problem. Please try.