[infinispan-dev] Infinispan's dependency injection fwk

[Galder Zamarreño]

On Jul 23, 2010, at 3:37 PM, Mircea Markus wrote:

> 
>>>> In terms of safe publishing, you can consider this safe since in the start method of the ComponentRegistry, internal volatile fields (including some locks) are touched, which means a JMM fence [1] is crossed.  
>>> Afaik safe publishing would only happen if *same* variables are touched by the user's thread accessing the object, is that the case?
>> 
>> Piggybacking is a valid strategy and is used in some core JDK constructs including AbstractQueuedSynchronizer, which is used to build locks, etc.  See Java Concurrency in Practice, section 16.1.4:
>> 
>> 	http://codeidol.com/java/java-concurrency/The-Java-Memory-Model/What-is-a-Memory-Model,-and-Why-would-I-Want-One/
>> 
>> More fun reading: 
>> 
>> 	http://gee.cs.oswego.edu/dl/jmm/cookbook.html
> Piggybacking still requires a sync/volatile variable to be touched by both threads in order for the publishing to to be correct (int the AbstractQueueSynchronizer/FutureTaks example, a volatile variable is set to null when reading/writing state, just to enforce happens-before).
> To be more specific, here the safe publishing issue that concerns me:
> T1 creates the Cache on DCM.getCache(). This calls @Inject method on CacheDelegate that would set CacheDelegate.invoker::InterceptorChain. N.B. invoker is not final nor volatile!
> T2 calls CacheDelegate.put, which uses the invoker field set by T1. What guarantee do we have that invoker was correctly published by T1 and seen by T2? 

Cache (or CacheDelegate) is not accessible to the client until DCM.getCache() has returned the Cache (CacheDelegate) instance back. Is your use case feasible? Could you claim that clients should make Cache volatile in this case? The way I read http://jeremymanson.blogspot.com/2008/11/what-volatile-means-in-java.html, you're gonna need Cache itself to be safely published between the two threads to make sure that what T1 writes is fully visible to T2. 

> Looking at the code, the safe publishing takes place in DefaultCacheManager.caches field: this is CHM where T1 added the cache and T2 got it from - so we're safe (published)!

Hmmm, caches is updated before cache.start which AFAIK, it's when @inject is called. So I don't think this would guarantee safe publishing. The CHM updated would need to happen afterwards if anything. However, the big question is, how does T2 access CacheDelegate? Does it take the Cache reference from DCM.getCache()? or does it get it from a volatile/non-volatile ref to Cache?

> One more detail, in the same class DefautlCacheManager.globalComponentRegistry doesn't seem to be correctly published (it is not a immutable object as well): this is created by T1 on new DCM and read by T2 when a Cache is instantiated, with no happens-before in between. I think we should declare it volatile.     

DefaultCacheManager.globalComponentRegistry is final, so that would be correct publishing AFAIK:

   private final GlobalComponentRegistry globalComponentRegistry;


>> 
> Cheers,
> Mircea
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache