Which kind of Singleton is this?

Question

public class ConnectionManager {

    private static Map <String, ConnectionManager> managerInstances = new HashMap<String, ConnectionManager>();

    private String dsKey;
    private ConnectionManager(String dsKey){
        this.dsKey = dsKey;
    }

    public static ConnectionManager getInstance(String dsKey){
        ConnectionManager managerInstance = managerInstances.get(dsKey);

        if (managerInstance == null) {
            synchronized (ConnectionManager.class) {
                managerInstance = managerInstances.get(dsKey);
                if (managerInstance == null) {
                    managerInstance = new ConnectionManager(dsKey);
                    managerInstances.put(dsKey, managerInstance);
                }
            }
        }
        return managerInstance;
    }
}

I recently saw this code somewhere where the Singleton pattern was not used as per book definition from GoF. Singleton is storing a Map of its own instances.

What kind of singleton can this be called? Or is this a valid use of Singleton?

@assylias While I won't disagree with (entirely ;)) an explanation might be nice — MadProgrammer, Aug 08 '13 at 06:27

score 9 · Accepted Answer · answered Aug 08 '13 at 06:26

9

It's not a singleton. It's called multiton pattern.

Rather than have a single instance per application, the multiton pattern instead ensures a single instance per key.

answered Aug 08 '13 at 06:26

kennytm

510,854
105
1,084
1,005

score 4 · Answer 2 · answered Aug 08 '13 at 06:30

4

That multiton seems broken as it uses a double-checked locking idiom which is not thread safe in Java. In particular, when calling getInstance(s), with s a non-null String, you could receive a non-null reference pointing to a ConnectionManager whose dsKey is null.

It would be much better to use a thread safe ConcurrentHashMap instead and remove the need for synchronization.

answered Aug 08 '13 at 06:30

assylias

321,522
82
660
783

1

As far as thread safety, it gets worse than that -- even if all the locking worked as the programmer thought it would. Time it just right, where one thread's getting from the map while another's writing to it, and the stars align, and [you can end up in an infinite loop](http://mailinator.blogspot.com/2009/06/beautiful-race-condition.html). It ain't just occasional lost or duplicate data we're talking about when we say "corruption", people. :P – cHao Aug 08 '13 at 06:47
@cHao Yes I had stopped looking before I had noticed that crucial point! – assylias Aug 08 '13 at 06:54
The double-checked locking can be made thread safe with `volatile`. See http://en.wikipedia.org/wiki/Double-checked_locking – mike Aug 08 '13 at 18:27
@mike That won't be enough in this case because (a) volatile will only apply to the map but not its content and (b) that does not solve the issue raised by cHao that if you put and get at the same time, the HashMap might become inconsistent (HashMap is not thread-safe). – assylias Aug 08 '13 at 18:29
I meant adding volatile to `ConnectionManager managerInstance` to avoid the problems that come with double-checked locking, i. e. ending up with two freshly created instances instead of one. – mike Aug 08 '13 at 18:37
@mike you can't add volatile to `ConnectionManager managerInstance`: it is a local variable. – assylias Aug 08 '13 at 18:38
What a pity. Hmm... maybe he can declare it as class variable `current`. Then at least the instance creation would be safe. And if `ConnectionManager` is properly synchronized, he wouldn't have to worry about the HashMap. ...That's all I have right now! – mike Aug 08 '13 at 18:42
@mike as long as the `get` is outside the synchronized block, all bets are off, even if you use a temporary volatile field to hold the result. I'm afraid there are only two (painless) ways: make the method synchronized or use a thread safe map implementation such as ConcurrentHashMap. – assylias Aug 08 '13 at 18:49
1

@assylias I provided a - what I hope to be - thread-safe implementation without the use of `ConcurrentHashMap` and `volatile`. Since it would be a little off-topic, I posted an own question http://stackoverflow.com/questions/18147113/thread-safe-multiton-pattern I would be happy, if you added your thoughts. – mike Aug 09 '13 at 12:54

Which kind of Singleton is this?

2 Answers2

Linked