Check double checked locking / Consider lazy initialiser or static initializer

[cfries]

At https://github.com/AlessandroGnoatto/mafinlib/blob/f45726047d589c93a603a8a879dd2a3bbff6f846/mafinlib/src/org/mafinlib/Settings.java#L142 the double checked locking idiom is used. This is broken in this form (since the member is non volatility). To fix it the member (singleton) should be volatility (also note that your singleton is not immutable)!

Background Information on Double Checked Locking

Double Checked Locking was broken in Java 1.4 and fixed in 1.5 (around 2005) using the keyword volatility. See https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html and http://www.angelikalanger.com/Articles/EffectiveJava/41.JMM-DoubleCheck/41.JMM-DoubleCheck.html Note that Double Checked locking is also broken in C++ and this is fixed in C++ 11 (around 2011), see http://preshing.com/20130930/double-checked-locking-is-fixed-in-cpp11/

Improvements

For lazy init I would recommend to use a standard LazyInitializer: https://commons.apache.org/proper/commons-lang/javadocs/api-3.1/org/apache/commons/lang3/concurrent/LazyInitializer.html implementing a ConcurrentInitializer: http://commons.apache.org/proper/commons-lang/javadocs/api-3.1/org/apache/commons/lang3/concurrent/ConcurrentInitializer.html

But for Singlestons lazy initialisation is trivial because you can use a static initializer - note that classed are not loaded until they are needed (Java does Lazy w.r.t. class loading) and a static initalizer is executed upon class loading - and thread safe by spec: https://stackoverflow.com/questions/3635396/pattern-for-lazy-thread-safe-singleton-instantiation-in-java

[AlessandroGnoatto]

Hi Christian,

Thanks for the links. They are very informative. I will fix those lines asap.

[AlessandroGnoatto]

I got the Impression that I do not need Double Check Locking at all, all I need to do is use synchronization. I now have something like

//Begin thread safe Singleton pattern implementation
private static Settings instance = null;

private Settings(){
    this.includeReferenceDateEvents = false;
    this.enforcesTodaysHistoricFixings = false;
}

public static synchronized Settings instance(){
    if(instance == null){
        instance = new Settings();
    }
    return instance;
}
//End thread safe Singleton pattern implementation

Do you think that synchronized might kill Performance? If this is the case I would go for the version with volatile. Thanks again

P.S. Should you have something similar in net.finmath.information.Library? I just realized that we are in a very similar setting there.