Details
-
Type:
Bug
-
Status:
Closed
-
Priority:
Major
-
Resolution: Won't Fix
-
Affects Version/s: 1.0-beta-3
-
Fix Version/s: None
-
Component/s: PicoContainer (Java)
-
Labels:None
-
Number of attachments :
Description
The method DefaultComponentAdapter.getComponentInstance is not thread-safe. The code block under "if (componentInstance == null)" suffers from the same issues as the "Double-Checked Locking" anti-pattern (http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html). Synchronization would need to be used to ensure the method will not return a half-baked (not fully initialized) component instance under certain conditions (e.g., with an optimizing compiler or on multi-processor machines). This issue also occurs in: BeanPropertyComponentAdapterFactory.Adapter.getComponentInstance, DefaultLifecyclePicoAdapter.initializeIfNotInitialized, InvokingComponentAdapterFactory.Adapter.getComponentInstance (this is not necessarily an exhaustive list).
Activity
I agree with Jon. Building thread safety into the core is a performance hog for applications that don't need it. A simple adapter wrapping the container could do it. Example:
public class ThreadSafeMutablePicoContainer {
private final MutablePicoContainer adaptee;
public ThreadSafeMutablePicoContainer(MutablePicoContainer adaptee)
{ this.adaptee = adaptee; }public synchronized Object getComponentInstance(Object componentKey)
{ return adaptee.getComponentInstance(componentKey); } ... etc ...
}
Would that work for you?
Aslak
Aslak Hellesøy
added a comment - I agree with Jon. Building thread safety into the core is a performance hog for applications that don't need it. A simple adapter wrapping the container could do it. Example:
public class ThreadSafeMutablePicoContainer {
private final MutablePicoContainer adaptee;
public ThreadSafeMutablePicoContainer(MutablePicoContainer adaptee)
{
this.adaptee = adaptee;
}
public synchronized Object getComponentInstance(Object componentKey)
{
return adaptee.getComponentInstance(componentKey);
}
... etc ...
}
Would that work for you?
Aslak Aslak Hellesøy
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Resolution | Won't Fix [ 2 ] | |
| Status | Unassigned [ 1 ] | Closed [ 6 ] |
From Jon Tirsen:
I think all thread-safety stuff should be handled by having thread-safe decorators to the PicoContainer interfaces that limits the number of threads inside the PicoContainer to one at a time.