Questo blocco doppio controllo è rotto?

Question

Checkstyle riporta questo codice come " L'idioma di blocco con doppio controllo è rotto " ;, ma non credo che il mio codice sia effettivamente influenzato dai problemi con il blocco con doppio controllo.

Il codice dovrebbe creare una riga in un database se non esiste una riga con quell'id. Funziona in un ambiente multi-thread e voglio evitare le eccezioni SQL della chiave primaria.

Lo pseudo-codice:

private void createRow(int id) {
  Row row = dao().fetch(id);
  if (row == null) {
     synchronized (TestClass.class) {
        row = dao().fetch(id);
        if (row == null) {
           dao().create(id);
        }
     }
  }
}

Posso essere d'accordo sul fatto che sembra un doppio controllo, ma non sto usando variabili statiche e il codice in fetch () e create () è probabilmente troppo complesso per essere incorporato e fuori servizio.

Ho torto o stile di controllo? :)

Answer 1

Supponendo che desideri leggere quella riga più interna:

row = dao().create(id);

Non è un classico problema di blocco con doppio controllo dato che dao (). fetch è correttamente silenziato dal metodo di creazione.

Modifica : (il codice è stato aggiornato)

Il classico problema di un blocco a doppio controllo consiste nell'assegnare un valore prima dell'inizializzazione in cui due thread accedono allo stesso valore.

Supponendo che il DAO sia correttamente sincronizzato e non restituirà un valore parzialmente inizializzato, ciò non risente dei difetti dell'idioma a doppio controllo.

Answer 2

Penso che in questo caso, checkstyle sia corretto. Nel tuo codice come presentato, considera cosa accadrebbe se entrambi i thread avessero entrambi row == null all'ingresso del blocco sincronizzato. Il thread A inserisce il blocco e inserisce la nuova riga. Quindi dopo che il thread A è uscito dal blocco, il thread B entra nel blocco (perché non sa cosa è appena successo) e prova a inserire di nuovo la stessa nuova riga.

Vedo che hai appena cambiato il codice e aggiunto una linea mancante piuttosto importante. Nel nuovo codice, potresti essere in grado di cavartela, poiché due thread non si baseranno sulle modifiche a una variabile condivisa (statica). Ma potresti essere meglio vedere se il tuo DBMS supporta un'istruzione come INSERT OR UPDATE .

Un altro buon motivo per delegare questa funzionalità al DBMS è se è necessario distribuire più di un server applicazioni. Poiché i blocchi sincronizzati non funzionano su più macchine, in ogni caso dovrai fare qualcos'altro.

Answer 3

Se sei tentato di scrivere codice in questo modo, considera:

• Da Java 1.4, i metodi di sincronizzazione sono diventati piuttosto economici. Non è gratuito ma il runtime in realtà non soffre così tanto che vale la pena rischiare la corruzione dei dati.

• Da Java 1.5, ci sono le classi Atomic * che consentono di leggere e impostare i campi in modo atomico. Sfortunatamente, non risolvono il tuo problema. Perché non hanno aggiunto AtomicCachedReference o qualcosa del genere (che chiamerebbe un metodo sostituibile quando viene chiamato get () e il valore corrente == null) è oltre me.

• Prova ehcache . Ti consente di impostare una cache (ovvero un oggetto che ti consente di chiamare il codice se una chiave è non contenuta in una mappa). Questo di solito è quello che vuoi e le cache risolvono davvero il tuo problema (e tutti quegli altri problemi che non sapevi nemmeno esistessero).

Answer 4

Come altri hanno sottolineato, questo codice farà ciò che si intende così com'è, ma solo in base a una serie rigorosa di presupposti non ovvi:

1. Il codice Java non è cluster (vedere la risposta di @Greg H)
2. La " riga " il riferimento è solo verificato per null nella prima riga, prima del blocco di sincronizzazione.

Il motivo per cui l'idioma di blocco ricontrollato è rotto (per la sezione 16.2.4 di Java Concurrency in Practice ) è che per un thread che esegue questo metodo è possibile vedere un riferimento non nullo ma inizializzato in modo errato a " row " ;, prima di inserire il blocco sincronizzato (a meno che "dao" fornisca una sincronizzazione corretta) . Se il tuo metodo stava facendo qualcosa con " row " oltre a controllare che sia nullo o no, sarebbe rotto. Allo stato attuale, probabilmente va bene ma molto fragile - personalmente non mi sentirei a mio agio nel commettere questo codice se pensassi che ci fosse anche una remota possibilità che qualche altro sviluppatore in un secondo momento potrebbe modificare il metodo senza comprendere le sottigliezze di DCL.