Details
-
Type: Improvement
-
Status: Closed
-
Priority: Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 1-1-beta-2
-
Component/s: None
-
Labels:None
-
Number of attachments :
Description
hi,
didn't want to commit this without having heard your opinions.
The current InstantiatingComponentAdapters aren't thread-safe because the instantiating/verifying booleans will confuse pico and throw CyclicDependencyException whenever 2 threads try to instantiate a component at the same time. (Demonstrated in http://cvs.picocontainer.codehaus.org/java/picocontainer/src/test/org/picocontainer/defaults/SynchronizedComponentAdapterTestCase.java?rev=1.4&view=auto)
However CyclicDependency usually only occur in testing/development since they either get eliminated by code changes or container magic. In live system its usually not needed to detect a cyclic dependency. Therefore I extracted this concern and moved it to a delegating ComponentAdapter which will take care of the detection.
One side-effect is that the CyclicDependencyException now reports the dependencies in the order they would have been instantiated and not everything that is on the failing component list. IMHO its way clearer and easier to understand where the cycle occurs.
To not break any expecting behavior before v1.0 we could have ConstructorInjectionComponentAdapterFactory wrap its Adapter in the CyclicDependencyDetectionAdapter by default.
Let me know what you think. Assign this to me if you want it to be applied.
(Patch + Adapter following).
PS: it might be possible to make cyclic dependencies go away if the Adapter would automatically return a dynamic proxy instead of the real component and hotswap it after the instantiation is done. Only for interfaces tho.
Activity
Field | Original Value | New Value |
---|---|---|
Attachment | patch.txt [ 12154 ] |
Attachment | CyclicDependencyDetectionAdapter.java [ 12155 ] |
Comment | [ 20005 ] |
Summary | Reworked CyclicDepency detection | Reworked CyclicDependency detection |
Comment | [ 20030 ] |
Fix Version/s | 1-1-beta-2 [ 11243 ] | |
Status | Open [ 1 ] | Closed [ 6 ] |
Resolution | Fixed [ 1 ] |
Comment | [ 30526 ] |
What would the consequences be of synchronizing on the component key - i.e. synchronize(compKey)
{ .. }instead of the soltion provided ?