Synchronized (Hash)Map of Singletons

Question

Description below the code...

// Singleton
public static final Map<String, Account> SHARED_ACCOUNT_HASHMAP =
        Collections.synchronizedMap(new HashMap<>());


public init(String[] credentials) {

    Account account = null;     

    String uniqueID = uniqueAccountIdentifier(credentials);

    if (SHARED_ACCOUNT_HASHMAP.containsKey(uniqueID)) {
        account = SHARED_ACCOUNT_HASHMAP.get(uniqueID);
        log("...retrieved Shared Account object: %s", uniqueID);
    }

    // create the Account object (if necessary)
    if (account == null) {
        account = new Account(credentials);

        // Store it in the SHARED_ACCOUNT_HASHMAP
        SHARED_ACCOUNT_HASHMAP.put(uniqueID, account);
        log("...created Account object: %s",uniqueID);

    }

}

What I want to achieve

There are multiple Threads accessing this Singleton HashMap
The goal of this HashMap is to only allow the creation of ONE Account per uniqueID
The account later can be retrieved by various threads for Account operations
Each Thread has this init() method and runs it once.
So the first Thread that cannot find an existing uniqueID Account, creates a new one and places it in the HashMap. The next Thread finds that for the same uniqueID, there is an Account object already - so retrieves it for its own use later

My problem...

How can I get the other Threads (second, third, etc.) to wait while the first Thread is inserting a new Account object?
to phrase it another way, there should never be 2 threads ever that receive a value of null when reading the HashMap for the same uniqueID key. The first thread may receive a value of null, but the second should retrieve the Account object that the first placed there.

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

According to the docs for synchronizedMap()

Returns a synchronized (thread-safe) map backed by the specified map. In order to guarantee serial access, it is critical that all access to the backing map is accomplished through the returned map.

It is imperative that the user manually synchronize on the returned map when iterating over any of its collection views

In other words you still need to have synchronized access to SHARED_ACCOUNT_HASHMAP:

public init(String[] credentials) {
    Account account = null;     
    String uniqueID = uniqueAccountIdentifier(credentials);

    synchronized (SHARED_ACCOUNT_HASHMAP) {
        if (SHARED_ACCOUNT_HASHMAP.containsKey(uniqueID)) {
            account = SHARED_ACCOUNT_HASHMAP.get(uniqueID);
            log("...retrieved Shared Account object: %s", uniqueID);
        }

        // create the Account object (if necessary)
        if (account == null) {
            account = new Account(credentials);

            // Store it in the SHARED_ACCOUNT_HASHMAP
            SHARED_ACCOUNT_HASHMAP.put(uniqueID, account);
            log("...created Account object: %s",uniqueID);
        }
    }
}

ah, I understand now - thanks. I did read in a tutorial about the Iterators, however I just ignored that since I didn't need to use Iterators. — ycomp, Oct 20 '15 at 05:26
With this implementation, you no longer need to use a synchronized map since only one thread will ever access it (for reading or writing) at a time. — Carlos Macasaet, Oct 20 '15 at 06:42

codejitsu · Answer 2 · 2015-10-20T05:38:09.030

1

Consider using ReadWriteLock if you have multiple readers/writers (see ReadWriteLock example).

Generally the ConcurrentHashMap performs better than the sinchronized hash map you are using.

edited Oct 20 '15 at 05:38

answered Oct 20 '15 at 05:22

codejitsu

3,162
2
24
38

I understand a bit about how `ConcurrentHashMap` works, but it sounds like overkill for what I need..I'll read up on ReadWriteLock - probably it will be helpful in other parts of my application. synchronizing on the hashmap as epoch suggested, is what I was looking for here. – ycomp Oct 20 '15 at 05:25
The `ReadWriteLock` will perform better than epoch's solution, but it does require considerably more code. – Carlos Macasaet Oct 20 '15 at 06:56
if I used `ConcurrentHashMap` would this `synchronized(on the hashmap)` block still be necessary? I think yes, right? my code won't work if I don't synchronize it that way - even with the `ConcurrentHashMap` – ycomp Oct 20 '15 at 16:29
You want to make two operations as one atomic call. In that case you have to use synchronization or special methods like http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/ConcurrentMap.html#putIfAbsent%28K,%20V%29 – codejitsu Oct 20 '15 at 16:36

score 1 · Answer 3 · edited May 23 '17 at 10:27

In the following code I can feel smell of race condition check-then-act as you are trying to perform two operations on the synchronised map (containsKey and get):

if (SHARED_ACCOUNT_HASHMAP.containsKey(uniqueID)) {
        account = SHARED_ACCOUNT_HASHMAP.get(uniqueID);
        log("...retrieved Shared Account object: %s", uniqueID);
 }

So to avoid race condition you need to synchronize over this map as:

synchronized (synchronizedMap) {
  if (SHARED_ACCOUNT_HASHMAP.containsKey(uniqueID)) {
            account = SHARED_ACCOUNT_HASHMAP.get(uniqueID);
            log("...retrieved Shared Account object: %s", uniqueID);
     }
  // rest of the code.      
}

Actually the synchronizedMap can protect itself against internal race conditions that could corrupt the map data but for external conditions (like above) you need to take care of that. If you feel you are using synchronized block at many places you can also think of using a regular map along with synchronized blocks. You will find this question also useful.

Synchronized (Hash)Map of Singletons

Description below the code...

What I want to achieve

My problem...

3 Answers3