Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Extending HashMap<K,V> and synchronizing only puts

I recently encountered a class on our code base that extends HashMap and synchronizes the put method.

Aside from it being less efficient than using ConcurrentHashMap, what sort of problems may arise with extending HashMap and synchronizing only put(K,V)?

Assume that we don't care whether get(K) returns the latest value or not(e.g. we are fine with threads overwriting each other and we don't care about the possible race conditions that may arise if the values of the map are used as locks themselves).

Example:

public class MyMap<K,V> extends HashMap<K,V> {
  //...
   public synchronized void put(K key, V value) {
      //...
   }

  //...
}

As far as I know, HashMap does its re-size with the put method and since the put is synchronized at the map instance level, the problems encountered during concurrent re-sizing will(probably) not be encountered.

Even with the questionable assumptions above, my gut-feeling is telling me that there are more problems that may arise. Or am I just being paranoid?

Update: Thank you all, that was fun and enlightening. If I ever meet the original of author of this particular class, I can now explain his folly in detail. :)

In summary: putAll can still screw up the data structure horribly and end up in the dreaded infinite-loop/data-race condition. get relies on the underlying internal data structures of hashmap that may be in the process of being modified concurrently causing the get process to behave weirdly. This is just a generally bad idea. At the very least, the author could have used Collections.synchronizedMap(Map) instead.

Note: All three answers given as of this writing are actually correct but I picked the one about get() as the correct answer since it was the least obvious one for me.

like image 652
Hyangelo Avatar asked Dec 26 '22 17:12

Hyangelo


2 Answers

since get() can be reading a changing data structure, everything bad can happen.

I've seen get() trapped in dead loop, so it's not just a theoretical possibility, bad things do happen.

like image 38
irreputable Avatar answered Jan 08 '23 06:01

irreputable


I hope you are also synchronizing on putAll and remove. putAll especially since multiple threads can try and resize your HashMap. Those methods too will be updating size and modCount which if done outside of synchronization can result in lost updates.

like image 121
John Vint Avatar answered Jan 08 '23 06:01

John Vint