0

Idea is to create a factory class that returns different singleton instances based on a 'type'. [Multiton pattern]. Also, the singleton instances should be lazily created.

Is the below code thread safe? Using ConcurrentHashMap makes it simpler but I want to try with HashMap. 

public class Multiton {

    private HashMap<String, Interface> interfaceMap;

    private static class Holder {
        public static final Multiton INSTANCE = new Multiton();
    }

    public static Multiton getInstance() {
        return Holder.INSTANCE;
    }

    private Multiton(){
        interfaceMap = new HashMap<String, Interface>();        
    }

    public Interface getInterface(String key){
        Interface value = interfaceMap.get(key);
        if(null == value){
            synchronized(interfaceMap){
                // double check locking
                if(interfaceMap.get(key) == null){
                    switch(key){
                    case TypeA : // initialize Type A interface here and put it to map
                        value = new TypeA();
                        interfaceMap.put(key, value);
                        break;
                    case TypeB : // initialize Type B interface here and put it to map
                        value = new TypeB();
                        interfaceMap.put(key, value);
                        break;
                    }
                }
            }
        }
        return interfaceMap.get(key);
    }
}
user170008
  • 1,046
  • 6
  • 17
  • 31

1 Answers1

0

Double checked locking is (in general) not thread safe. In particular, it is not sufficient to only synchronize when writing to a map, as that does not prevent a reader thread from accessing the state a writer thread is currently modifying.

If you can't use a ConcurrentHashMap (for instance because you must ensure that a single TypeA is created, not just that a single TypeA is used by all clients), the following should work and be reasonably cheap. In particular, it is lock free once the lazy object has been constructed.

abstract class Lazy<T> {
    private volatile T instance;

    abstract T create();

    void get() {
        if (instance == null) {
            synchronized (this) {
                if (instance == null) {
                    instance = create();
                }
            }
        }
        return instance;
    }
}

class Multiton {
    private final Map<String, Lazy<?>> map = new HashMap<>();

    Multiton() {
        map.put("A", new Lazy<A> {
            A create() {
                return new A();
            }
        }
        map.put("B", new Lazy<B> {
            B create() {
                return new B();
            }
        }
        // and so on for the other types
    }
}

For production quality, you might want to consider using lamdba expressions for the factory methods, and type safe keys for the map (such as the class objects of the interface requested).

meriton
  • 68,356
  • 14
  • 108
  • 175
  • This was correct for Java 1.4 and earlier. Since 1.5 DCL works correctly. I don't want to downvote you, but this answer is wrong and you should remove it. – Kayaman Aug 27 '14 at 22:39
  • The article I linked to clearly states that double checked locking is correct only under very specific circumstances. It is not correct in this case. – meriton Aug 27 '14 at 22:52
  • The article you linked to is ancient. Since the improved memory model of Java 1.5, the article is obsolete. – Kayaman Aug 27 '14 at 22:57
  • 1
    If you had read to the end, you might have spotted the section "Under the new Java Memory Model", which correctly describes that starting from Java 5, double checked locking is only safe if the singleton is accessed through a volatile field to establish a happens-before relationship between assignment and access of the singleton. – meriton Aug 27 '14 at 23:07
  • You may be right, it's late and the article describes a singleton, not a multipleton. – Kayaman Aug 27 '14 at 23:10