Update map/set and check size atomically

Question

I have the following method:

private static HashSet<Integer> ids = new HashSet<>();  
public static void someMethod(SomeObject o) {  
  // some code  

 ids.add(o.getId());   
 if(ids.size() > 10) {  
   // do something  
 }  
 else {  
  // do something else  
 }  
}

An easy way to make this method thread safe is to add the keyword synchronized.
I was wondering if there is already some more appropriate way to add an item in a map/set and check size atomically

Don't know any other possibility than using `synchronized` keyword. Is there something wrong using it? — AnarchoEnte, Sep 05 '18 at 13:12
@AnarchoEnte: I was only wondering if there is another way via the `ConcurrentHashMap` and similar — Jim, Sep 05 '18 at 13:13
Following this post you cannot use a `ConcurrentHashMap` to reach full thread saftey for your code above due to the `ids.size()` call. https://stackoverflow.com/questions/25998536/does-a-concurrenthashmap-need-wrapped-in-a-synchronized-block — AnarchoEnte, Sep 05 '18 at 13:18
`ConcurrentHashMap` is *not* a solution, it will return an estimate of the size. But if you *synchronize* the entire method here you would be doing a lot more than *add an item in a map/set and check size atomically*, you would also do `do something` or `do something else` - you understand that right? — Eugene, Sep 05 '18 at 14:11
@Eugene: Yes but these parts are of minor performance impact. I was wondering if there is an idiomatic to java way of addressing this — Jim, Sep 05 '18 at 16:25

score 1 · Answer 1 · answered Sep 06 '18 at 10:11

Such a thing does not exist, as said in the comments not even ConcurrentHashMap will help you.

Be very careful here not fall in the trap of the check-than-act anti-pattern here. So for example not to do:

 Set<Integer> syncIds = Collections.synchronizedSet(ids);
 syncIds.add(o.getId())  
 if(ids.size() > 10) {....}

As separate operations, these are indeed thread-safe, but as compound ones, they are are not.

Bare in mind that since you update that HashSet under a lock, when reading it, it has to be done under the same lock, this is how happens-before works. So if you expose a method:

public Set<Integer> getIds(){
    return ids;
}

For this to work correct and you don't run into surprises, this method needs to be synchronized too.

Update map/set and check size atomically

1 Answers1