0

I want to increase a counter for a ConcurrentHashmap, but the key can be deleted at any time by another thread. So I check if the key still exists before increasing the counter.

I have the following code:

if(this.myMap.containsKey(key))
{
    this.myMap.put(key, this.myMap.get(key) + 1);
}

myMap is created like this:

private Map<String, Integer> myMap = new ConcurrentHashMap<String, Integer>();

But from time to time I get a NullPointer Exception in the line where I increase the integer. This doesn't happen often (maybe 1 out of 10 Million runs).

Is there another way to increase the counter? Or Do I even need a HashMap?

modsfabio
  • 985
  • 1
  • 10
  • 22
  • This is not a duplicate.. Did you even read my question? I know what a Nullpointer is, that's why I check if the key exists. – modsfabio May 30 '17 at 16:57
  • 1
    did you ever consider that your check happens and then the entry gets deleted before you do the `.get()` that is what happens with concurrent code. `ConcurrentHashMap` does not guarantee what you think it does. –  May 30 '17 at 16:58
  • That's exactly my problem, and I thought you would help me out. Edit: Thanks @davidxxx, is there a simple way to solve this? – modsfabio May 30 '17 at 16:59
  • read for comprehension the answers to **ALL** the duplicates listed –  May 30 '17 at 17:00
  • Of course, synchronize the set of instructions with the `synchronized` keyword – davidxxx May 30 '17 at 17:01
  • @davidxxx - that will not stop another thread from removing something, that is a completely naive *solution* that is not a solution, concurrency is much more than slinging `synchronized` keywords around randomly until it *works* until it does not work. –  May 30 '17 at 17:01
  • @Jarrod Roberson Synchronize the map instance will not be enough ??? – davidxxx May 30 '17 at 17:02
  • so how does putting the `contains` and `get` in a `synchronized` block stop another thread from calling `remove` between their execution? –  May 30 '17 at 17:03
  • You don't answer to the question. The synchronized statement on the map instance will prevent NPE and inconsistency. The removal from another thread will happen **before** or **after** the synchronized statement. – davidxxx May 30 '17 at 17:06
  • If you're using Java 8, you're going to need to use `computeIfPresent.` If not, you'll need to do a `replace` loop. – Louis Wasserman May 30 '17 at 17:08
  • What about this? 
    private Map myMap = new ConcurrentHashMap();
    
    public void add(String key){
        AtomicInteger ai = myMap.get(key);
        if (ai != null) ai.addAndGet(1);
    }
    – tsolakp May 30 '17 at 18:43
  • What, if I use this: `this.myMap.put(key, this.myMap.getOrDefault(key, 0) + 1);` Is it safe to use? Additionally I don't have to check if it exists. – modsfabio May 30 '17 at 20:00

0 Answers0