Java Singleton sincronizzazione per il multi-thread utilizzando HashMap

Question

ho la seguente classe:

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); 
    } 

ho pensato che sarebbe stato sufficiente per evitare l'accesso simultaneo, ma ho ottenuto questo errore:

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

I ha 10 thread che usano questa classe e lancia questo errore circa una volta ogni 100.000 chiamate.

Che cosa è sbagliato con questo singleton?

Answer 1

Il problema è semplicemente che HashMaps non sono thread-safe come puoi leggere nei documenti collegati.

Provare a cambiarli in ConcurrentHashMaps.

A parte questo, è necessario modificare l'implementazione di singleton per gestire meglio il multi threading. La pagina di Wikipedia su Double-checked locking fornisce molti buoni esempi.

p.s .: Invece di dichiarare le variabili come HashMaps, dovresti semplicemente dichiararle come Maps. In questo modo è possibile modificare l'implementazione specifica molto facilmente senza dover rifattorizzare nulla. Questo è chiamato Programming to interfaces.

Answer 2

Credo che il problema è con HashMap, utilizzare Concurrent HashMap

Ma il punto che voglio fare è la funzione getInstnace() non è ben scritto.

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

Si utilizza synchronized su tutto il metodo. Anche se la tua istanza è stata creata, solo una discussione sarà in grado di entrare nel metodo getInstance, che rallenta le prestazioni del tuo programma. Si dovrebbe fare come this:

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

     return instance; 
    }