2014-05-22 5 views
15

Ho bisogno di inizializzare pigramente una mappa e il suo contenuto. Ho il codice di seguito fino ad ora:Inizializza tranquillamente una mappa Java in modo thread-safe

class SomeClass { 
    private Map<String, String> someMap = null; 

    public String getValue(String key) { 
     if (someMap == null) { 
      synchronized(someMap) { 
       someMap = new HashMap<String, String>(); 
       // initialize the map contents by loading some data from the database. 
       // possible for the map to be empty after this. 
      } 
     } 
     return someMap.get(key); // the key might not exist even after initialization 
    } 
} 

Questo ovviamente non è thread-safe, come se un thread viene quando someMap è nullo, va avanti per inizializzare il campo per new HashMap e mentre il suo ancora il caricamento dei dati nella mappa , un altro thread fa un getValue e non ottiene i dati quando uno potrebbe essere esistito.

Come posso verificare che i dati vengano caricati nella mappa una sola volta quando si verifica la prima chiamata getValue.

Si noti che è possibile che lo key non esista nella mappa dopo l'inizializzazione. Inoltre, è possibile che la mappa sia semplicemente vuota dopo l'inizializzazione.

+13

inoltre riceverete NullPointerException sulla linea 'sincronizzato (someMap)' 'dal someMap' è' null' – Idolon

+3

In questa discussione, le persone che non si rendono conto quanto sia difficile concomitante è. –

risposta

25

Double Check Blocco

Doppia chiusura di controllo richiede diversi passaggi tutti da completare per poter funzionare correttamente, vi manca due di loro.

Per prima cosa è necessario convertire someMap in una variabile volatile. Questo è così che gli altri thread vedranno le modifiche apportate ad esso quando sono fatti ma dopo che le modifiche sono state completate.

private volatile Map<String, String> someMap = null; 

è necessario anche un secondo controllo per null all'interno del blocco synchronized per fare in modo che un altro thread non è inizializzato per voi mentre erano in attesa di entrare nella zona sincronizzato.

if (someMap == null) { 
     synchronized(this) { 
      if (someMap == null) { 

non si assegna fino al momento dell'uso

Nella vostra generazione della mappa costruirlo in una variabile temporanea quindi assegnare alla fine.

   Map<String, String> tmpMap = new HashMap<String, String>(); 
       // initialize the map contents by loading some data from the database. 
       // possible for the map to be empty after this. 
       someMap = tmpMap; 
      } 
     } 
    } 
    return someMap.get(key); 

Per spiegare perché è necessaria la mappa temporanea. Non appena si completa la linea someMap = new HashMap... allora someMap non è più nullo. Ciò significa che altre chiamate a get lo vedranno e non tenteranno mai di inserire il blocco synchronized.Proveranno quindi ad ottenere dalla mappa senza attendere il completamento delle chiamate al database.

Assicurandosi che l'assegnazione a someMap sia l'ultimo passaggio all'interno del blocco sincronizzato che impedisce che ciò accada.

unmodifiableMap

Come discusso nei commenti, per la sicurezza sarebbe anche meglio per salvare i risultati in un unmodifiableMap come future modifiche non sarebbero stati al sicuro thread. Questo non è strettamente necessario per una variabile privata che non viene mai esposta, ma è ancora più sicura per il futuro in quanto impedisce alle persone di entrare in seguito e di cambiare il codice senza rendersene conto.

  someMap = Collections.unmodifiableMap(tmpMap); 

Perché non utilizzare ConcurrentMap?

ConcurrentMap rende azioni individuali (cioè putIfAbsent) thread-safe, ma non soddisfa il requisito fondamentale qui di attendere la mappa è completamente popolato con i dati prima di consentire legge da esso.

Inoltre, in questo caso, la mappa dopo l'inizializzazione pigra non viene modificata di nuovo. ConcurrentMap aggiungerebbe sovraccarico di sincronizzazione alle operazioni che in questo caso d'uso specifico non devono essere sincronizzate.

Perché sincronizzare su questo?

Non c'è motivo. :) Era il modo più semplice per presentare una risposta valida a questa domanda.

Sarebbe sicuramente meglio eseguire la sincronizzazione su un oggetto interno privato. Hai migliorato l'incapsulamento scambiato per l'utilizzo di memoria marginalmente aumentato e tempi di creazione dell'oggetto. Il rischio principale con la sincronizzazione su this è che consente ad altri programmatori di accedere all'oggetto di blocco e potenzialmente provare a sincronizzarsi su di esso. Ciò causa quindi un conflitto non necessario tra i loro aggiornamenti e il tuo, quindi un oggetto di blocco interno è più sicuro.

In realtà un oggetto di blocco separato è eccessivo in molti casi. È un giudizio basato sulla complessità della tua classe e su quanto ampiamente viene utilizzato contro la semplicità del solo blocco su this. In caso di dubbio, probabilmente dovresti usare un oggetto di blocco interno e prendere la via più sicura.

Nella classe:

private final Object lock = new Object(); 

Nel metodo:

synchronized(lock) { 

Per quanto riguarda java.util.concurrent.locks oggetti, non aggiungono nulla di utile in questo caso (anche se in altri casi sono molto utili). Vogliamo sempre aspettare che i dati siano disponibili, quindi il blocco sincronizzato standard ci fornisce esattamente il comportamento di cui abbiamo bisogno.

+1

Questo è l'idioma più comune. Consigliato. Vorrei usare la mappa immutabile di Guava se i dati non sono soggetti a modifiche. –

+0

@GiovanniBotta Se guava non è disponibile 'Collections.unmodifiableMap' è il più comune che ho visto. –

+0

@JohnVint In questo caso, in realtà, se la mappa è privata ei valori non cambiano mai [non è importante] (http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/collect /ImmutableMap.html). –

1

Il motivo per cui si desidera inizializzare la mappa è pigro perché la generazione di valori richiede un uso intensivo delle risorse. Generalmente è possibile distinguere tra due casi d'uso

  1. generazione/memorizzazione di ciascun valore è altrettanto costosa
  2. generazione di valori è costoso, ma se si genera uno, generando il resto non è più costoso (per esempio è necessario interrogare un database)

Il Guava library ha una soluzione per entrambi. Utilizzare un valore Cache per generare valori al volo o un valore + loadAll per generare valori. Poiché l'inizializzazione di una cache vuota è praticamente gratuita, non è necessario utilizzare double check idiom: è sufficiente assegnare l'istanza di Cache a un campo private final.

+0

Capisco una cache a prima vista, questo aiuta ad aggiungere elementi alla cache, ma non a creare la cache pigramente in primo luogo. –

+0

@Silly Freak La cache viene creata pigra mentre invocate [get] (http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/cache/Cache.html) – ooxi

+0

I'm parlando di 'Cache c = null; c.get (chiave); 'lancio di un NPE. L'OP chiedeva di creare in modo sicuro la cache/mappa, non inserendovi. –

-6

È possibile utilizzare il codice seguente per ottenere la mappa sincronizzata con blocco livello di classe.

Mappa aSynchronizedMap = Raccolte.synchronizedMap (new HashMap());

class SomeClass { 
    private Map<String, String> someMap = null; 

    public String getValue(String key) { 
     if (someMap == null) { 
      synchronized (SomeClass.class) { 

        someMap = Collections.synchronizedMap(new HashMap<String, String>()); 
       // initialize the map contents by loading some data from the database. 
       // possible for the map to be empty after this. 
      } 
     } 
     return someMap.get(key); // the key might not exist even after initialization 
    } 
} 
+2

la mappa stessa può essere sincronizzata, ma la creazione della mappa non lo è. nemmeno il popolamento della mappa è atomico –

+2

Questo thread non è sicuro: la mappa può essere creata e inizializzata più volte da thread diversi (non dal downfer). – OldCurmudgeon

2

Penso TimB spiegato maggior parte delle opzioni molto bene, ma penso che la risposta più veloce e più ovvia sarebbe quella di creare quando l'istanza della classe viene creata un'istanza.

class SomeClass { 
    private final Map<String, String> someMap = new HashMap<String, String>(); 

    public String getValue(String key) { 
     return someMap.get(key); // the key might not exist even after initialization 
    } 
}