Complex chains of circular dependencies makes Typhoon to inject nil

[cesteban]

ComponentA is singleton and uses init injection, ComponentB is prototype and uses property injection.

1.- Some component asks for ComponentA: ComponentA is marked as currently resolving, and Typhoon initiates its instantiation.
2.- ComponentA has a circular dependency on ComponentB, correctly detected.
3.- Typhoon tries to build ComponentB. It has a dependency on ComponentA, detected as circular (and left for later), and another dependency on ComponentC which in turn depends on ComponentA again!
4.- When building ComponentC, ComponentA must be built, so it is allocated and marked as currently resolving once more -> This overwrites the dictionary value of ComponentA!!!
5.- When ComponentA is ready (correctly injected with the still not complete instance of ComponentB), it is injected in ComponentC and marked as done resolving -> This means the only instance of ComponentA that we still had in the registry is deleted!
6.- ComponentC is ready and injected in ComponentB. Now it is turn of circular dependencies of B, which includes A. Typhoon injects the instance of A that is supposed to be stashed in _currentlyResolvingReferences
7.- [_currentlyResolvingReferences objectForKey:componentKey] returns nil
8.- ComponentB is injected with a nil.

_currentlyResolvingReferences must be promoted to act as a temp stash local to the current cycle. I believe @jasperblues was asking for something like this in #57.

I'm working on a fix for this issue, I'll let you know. I tried to build a failing test but it is very difficult to reproduce using simple classes. Furthermore, the bug only appears sometimes, as the order in which assembly definitions are instantiated is not defined, and has a great influence.

[jasperblues]

Sounds like a tragic scene in a spaghetti western! ;) . . . look forward to the fix! :)

[jasperblues]

By allowing circular deps with prototype scope to participate as initializer arguments, we're definitely moving away from Spring territory here. . . the price might be that we have to make the factory methods on TyphoonComponentFactory (eg componentForKey, componentForType) @synchronized . .

Either that or use one stash per request.

[jasperblues]

Will the new/pending "weak singleton" scope address your issue?

The idea behind "weak singleton" is that its the same as singleton, except it goes away (ie dealloc'd) when nothing else besides Typhoon is using it. . . (except during instantiation of course, when it would have to be a strong reference).

[cesteban]

No, this is a bug, we should not inject nils, no matter the scope of the components. Fix is very easy: use a little bit more complex structure for the _currentlyResolvingDefinition: a dictionary of stacks. For each key we hold and stack, so every component that asks for a prototype dependency has its own instance allocated and stashed during its building. Then the instance is popped, not disrupting other currently resolving references.

[jasperblues]

If we go this way what happens with calls from multiple threads to [factory componentForKey:] . . . ? I think we'll need to make it @synchronized.

[jasperblues]

is this closed?

[jasperblues]

@cesteban Perhaps to test we could:

• Loop 1000 times. If all pass test passes.

[cesteban]

Not closed yet. We need to address the concurrency issues and polish a little bit.

[cesteban]

@jasperblues that test you suggest would be unstable, which means useless. No, I need to find an STR, but it is kind of tricky, and I don't think it is a priority right now. What do you think?

[jasperblues]

Agree - not a current priority.