I browsed Guice's sources some and I might some idea of where that bug
is. The line InjectorImpl.java:672 calls
com.google.inject.InjectorImpl.injectors.get(Object) whose
implementation is com.google.inject.util.AbstractReferenceCache#get.
The only possibility of AbstractReferenceCache#get to return null is
if AbstractReferenceCache#internalCreate returns null. The only
possibility for that to return null is if the line "return previous.get
();" returns null, in other words if
com.google.inject.util.AbstractReferenceCache.FutureValue#get returns
null.

Having a closer look at FutureValue showed a synchronization issue. In
FutureValue.get(), the access to fields 'set', 't' and 'value' is not
synchronized. This looks similar to the kind of concurrency bug in a
typical wrongly implemented double-checked-locking:
http://jeremymanson.blogspot.com/2008/05/double-checked-locking.html
http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

Now that I look at FutureValue even closer, it's not even enough to
mark the fields 'set', 't' and 'value' volatile, like in the case of
double-checked-locking, because the following is possible:
1. Thread 1 calls setValue(someValue), which executes "set();"
completely, which sets "set = true".
2. Thread 2 calls get(), notices that "set == true", and returns
"value" which is null.
3. Thread 1 sets "value = someValue".

To fix that, the FutureValue.get() method should be marked
synchronized.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"google-guice" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/google-guice?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to