[Biojava-dev] Thread safety question

[Thomas Down]

Once upon a time, Keith James wrote:
> 
> I was looking at lazy instantiation and thread safety issues. There
> are a few well-publicised articles about the double checked locking
> idiom in Java and how it doesn't quite work e.g.
> 
> http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
> 
> This rang a bell regarding AbstractChangeable which does this
> 
> protected ChangeSupport getChangeSupport(ChangeType ct) {
>     if(changeSupport != null) {
>       return changeSupport;
>     }
> 
>     synchronized(this) {
>       if(changeSupport == null) {
>         changeSupport = generateChangeSupport();
>       }
>     }
> 
>     return changeSupport;
>   }
> 
> The example given in the reference is
> 
> // Broken multithreaded version
> // "Double-Checked Locking" idiom
> class Foo {
>   private Helper helper = null;
>   public Helper getHelper() {
>     if (helper == null)
>       synchronized(this) {
>         if (helper == null)
>           helper = new Helper();
>       }
>     return helper;
>     }
>   // other functions and members...
>   }
> 
> where the initial test is reversed wrt the biojava example. However,
> it looks to me like there might be a (theoretical) danger here (of
> ChangeSupport being initialised twice). Can someone convince me
> otherwise?

Yes, I think the problem could occur here.

That pattern, or similar, gets used quite a bit in BioJava.  Many
of the cases (for example, in the Ensembl wrappers) are harmless
because the worst that can happen is that data gets loaded twice.
But for ChangeSupport there is a real problem, if a listener gets
added to the first ChangeSupport, which is then overwritten by
a second.

I seem to remember that it's possible to fix this by declaring the
relevant field volatile -- is this true?

[I think there are also some memory model changes proposed which
are supposed to fix this, but I don't know if/when they'll actually
happen]

     Thomas.