Java Singleton Sync for Multithreaded Using HashMap

Question

I have the following class:

public class AggregationController { private HashMap<String, TreeMap<Integer, String>> messages; private HashMap<String, Integer> counters; Boolean buildAggregateReply; private boolean isAggregationStarted; private static HashMap<String, AggregationController> instances = new HashMap<String, AggregationController>(); private AggregationController() throws MbException{ messages = new HashMap<String, TreeMap<Integer,String>>(); counters = new HashMap<String, Integer>(); buildAggregateReply = true; isAggregationStarted = false; } public static synchronized AggregationController getInstance(String id) throws MbException{ if(instances.get(id) == null) instances.put(id, new AggregationController()); return instances.get(id); } 

I thought this was enough to avoid concurrent access, but I got this error:

 HashMap.java checkConcurrentMod java.util.HashMap$AbstractMapIterator java.util.ConcurrentModificationException Unhandled exception in plugin method java.util.ConcurrentModificationException 

I have 10 threads using this class, and it throws this error about once every 100,000 calls.

What is wrong with this single?

Answer 1

The problem is that HashMaps are not thread safe, as you can read in related documents.

You should try changing them to ConcurrentHashMaps .

In addition, you should also change your singleton implementation to better handle multithreading. The Wikipedia page on Double Lock Check contains many good examples.

ps: Instead of declaring your variables as HashMaps, you should simply declare them as Maps. That way, you can very easily change a particular implementation without reorganizing anything. This is called Interface Programming .

Answer 2

I think the problem is in HashMap , use Parallel HashMap

But the point I want to make is your getInstnace() function is getInstnace() incorrectly.

  public static synchronized AggregationController getInstance(String id) throws MbException{ if(instances.get(id) == null) instances.put(id, new AggregationController()); return instances.get(id); } 

You are using synchronized for the whole method. Even if your instance is created, only one thread can enter the getInstance method, which slows down the performance of your program. You need to do this :

  public static AggregationController getInstance() { if (instance == null ) { synchronized (AggregationController.class) { if (instance == null) { instance = new AggregationController(); } } } return instance; }