Inconsistent responses when using ConcurrentHashMap in multi-threaded environment

Question

We have a single thread that regularly updates a Map. And then we have multiple other threads that read this map.

This is how the update thread executes

private Map<String, SecondMap> firstMap = new ConcurrentHashMap<>();

private void refresh() //This method is called every X seconds by one thread only
{
   List<SecondMap> newData = getLatestData();
   final List<String> newEntries = new ArrayList<>(); 
   for(SecondMap map : newData) {
       newEntries.add(map.getName());
       firstMap.put(map.getName(), map); 
   }
   final Set<String> cachedEntries = firstMap.keySet();
   for (final String cachedEntry : cachedEntries) {
       if (!newEntries.contains(cachedEntry)) {
           firstMap.remove(cachedEntry);
       }
   } 
}

public Map<String, SecondMap> getFirstMap()//Other threads call this
{
    return firstMap;
}

The SecondMap class looks like this

class SecondMap {
    Map<String, SomeClass> data; //Not necessarily a concurrent hashmap
    public Map<String, SomeClass> getData() {
        return data;
    }
}

Below is the simplified version of how reader threads access

public void getValue() {
    Map<String, SecondMap> firstMap = getFirstMap();
    SecondMap secondMap = firstMap.get("SomeKey");
    secondMap.getData().get("AnotherKey");// This returns null
}

We are seeing that in other threads, when they iterate over the received firstMap, sometimes they get null values for some keys in the SecondMap. We don't see any null values for keys in the firstMap, but we see null values for keys in second value. One thing that we can rule out is that the method getLatestData will never return such data. It reads from a database and returns these entries. There can never be null values in the database in the first place. Also we see that this happens occasionally. We are probably missing something here in handling multi-threaded situation in a proper way, but I am looking for an explanation why this can happen.

Can you post the complete code that calls getFirstMap()? Specifically, the part that the sentence "when they iterate over the received firstMap, sometimes they get null values for some keys in the SecondMap" refers to would help. — Yoav Gur, Oct 11 '18 at 10:47
Are you sure you use proper synchronization both when reading and writing (https://stackoverflow.com/questions/2787094/how-to-demonstrate-java-multithreading-visibility-problems)? I just can't say much with some parts of the code snippet hidden. ConcurrentHashMap already encapsulates synchronization policy, but the situation is a little bit more complex if there are inner non-concurrent data structures being used. — nyarian, Oct 11 '18 at 11:03
@YoavGur and AndreyIlyunin - I added more details if it can help — Shiva Kumar, Oct 11 '18 at 11:19
Assuming that this is all the relevant code, the only methods that touches any SecondMap instance are getName() and getData(). Therefore, one must conclude that either the key 'AnotherKey' was not returned in getLatestData(), or that one of the methods getName() and getData() changes the underlying data. — Yoav Gur, Oct 11 '18 at 12:02
Purely theoretically, the memory model allows for reordering of objects assignments and initialization between threads (Meaning another thread may get an object before it is fully initialized), but I find it very hard to believe that this is the case here. You could turn getLatestData() to synchronized, just to rule it out. — Yoav Gur, Oct 11 '18 at 12:35
When you run `getLatestData()` are you modifying the existing `SecondMap` or are you creating new instances of `SecondMap`? — John Vint, Oct 11 '18 at 12:52
@JohnVint New instances of SecondMap are created(actually getLatestData() is a response from DynamoDB scan) — Shiva Kumar, Oct 11 '18 at 13:29
Ok, you will want to rule out if it is a memory-model concern. Based on what you said and what I see in the code I would doubt it is. https://pastebin.com/RKDdPZja. Take a look at that. You will want to add `synchronized` to the method, then, if/when the `AnotherKey` get returns null, try to get it again around `synchronized(this)`. This assumes that both `getValue` and `refresh` are under the same object. With the CHM you are hoping for a happens-before relationship, if you are not getting it, you can try to give yourself a more robust one by using `synchronized`. — John Vint, Oct 11 '18 at 13:54

score 0 · Answer 1 · answered Oct 13 '18 at 06:58

Assuming the Map<String, SomeClass> data; inside the SecondMap class is a HashMap, you can get a null value for a key in two scenarios. 1. If the key maps to a null value. Example "Something" -> null. 2. If the key is not in the map in the first place.

So without knowing much about where the data is coming from. If one of maps returned by getLatestData(); doesn't have the key "SomeKey" in the map at all, it will return null.

Also since there's not enough information about how that Map<String, SomeClass> data; is updated, and if it's mutable or immutable, you may have issues there. If that map is immutable and the SecondMap is immutable then it's more probably ok. But if you are modifying if from multiple threads you should make it a ConcurrentHashMap and if you update the reference to a new Map<String, SomeClass> data from different threads, inside the SecondMap you should also make that reference volatile.

class SecondMap {
    volatile Map<String, SomeClass> data; //Not necessarily a concurrent hashmap
    public Map<String, SomeClass> getData() {
        return data;
    }
}

If you'd like to understand in depth on when to use the volatile keyword and all the intricacies of data races, there's a section in this online course https://www.udemy.com/java-multithreading-concurrency-performance-optimization/?couponCode=CONCURRENCY about it. I have not seen any resource that explains and demonstrates it better. And unfortunately there are so many articles online that just explain it WRONG, which is sad.

I hope from the little information in the question I was able to point you to some directions that might help. Please share more information if nothing of that works, or if something does work, please let me know, I'm curious to know what it was :)

Inconsistent responses when using ConcurrentHashMap in multi-threaded environment

1 Answers1