Qual è il problema di concorrenza più frequente che hai incontrato in Java?

Questo è un sondaggio sui problemi di concorrenza comuni in Java. Un esempio potrebbe essere il classico deadlock o race condition o forse bug di thread EDT in Swing. Sono interessato sia ad una vasta gamma di possibili problemi, ma anche a quali sono i problemi più comuni. Quindi, per favore lascia una risposta specifica di un bug di concorrenza Java per commento e vota se vedi quello che hai incontrato.

Il problema di concorrenza più comune che ho visto, non si sta rendendo conto che un campo scritto da un thread non è garantito per essere visto da un thread diverso. Un’applicazione comune di questo:

class MyThread extends Thread { private boolean stop = false; public void run() { while(!stop) { doSomeWork(); } } public void setStop() { this.stop = true; } } 

Finché stop non è volatile o setStop e run non sono sincronizzati, non è garantito che funzioni. Questo errore è particolarmente diabolico, dato che nel 99,999% non importa in pratica poiché il thread del lettore vedrà finalmente il cambiamento – ma non sappiamo quanto presto lo abbia visto.

Il mio problema di concorrenza più doloroso n. 1 si è mai verificato quando due diverse librerie open source hanno fatto qualcosa del genere:

 private static final String LOCK = "LOCK"; // use matching strings // in two different libraries public doSomestuff() { synchronized(LOCK) { this.work(); } } 

A prima vista, questo sembra un esempio di sincronizzazione piuttosto banale. Però; Poiché le stringhe sono internate in Java, la stringa letterale "LOCK" risulta essere la stessa istanza di java.lang.String (anche se sono dichiarate completamente diverse l’una dall’altra). Il risultato è ovviamente negativo.

Un classico problema sta cambiando l’object su cui stai sincronizzando durante la sincronizzazione su di esso:

 synchronized(foo) { foo = ... } 

Altri thread concorrenti si sincronizzano su un object diverso e questo blocco non fornisce l’esclusione reciproca prevista.

Un problema comune è l’utilizzo di classi come Calendar e SimpleDateFormat da più thread (spesso memorizzandoli nella cache in una variabile statica) senza sincronizzazione. Queste classi non sono thread-safe, quindi l’accesso multi-thread alla fine causerà strani problemi con lo stato incoerente.

Blocco a doppio controllo. Nell’insieme.

Il paradigma, che ho iniziato ad apprendere i problemi di quando lavoravo in BEA, è che le persone controllano un singleton nel modo seguente:

 public Class MySingleton { private static MySingleton s_instance; public static MySingleton getInstance() { if(s_instance == null) { synchronized(MySingleton.class) { s_instance = new MySingleton(); } } return s_instance; } } 

Questo non funziona mai, perché un altro thread potrebbe essere entrato nel blocco sincronizzato e s_instance non è più nullo. Quindi il cambiamento naturale è quindi di farlo:

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

Anche questo non funziona, perché il modello di memoria Java non lo supporta. Devi dichiarare che s_instance è volatile per farlo funzionare, e anche allora funziona solo su Java 5.

Le persone che non hanno familiarità con le complessità del modello di memoria Java lo fanno incasinare tutto il tempo .

Sincronizzazione non corretta sugli oggetti restituiti da Collections.synchronizedXXX() , in particolare durante l’iterazione o più operazioni:

 Map map = Collections.synchronizedMap(new HashMap()); ... if(!map.containsKey("foo")) map.put("foo", "bar"); 

È sbagliato . Nonostante le singole operazioni siano synchronized , lo stato della mappa tra invocazione contains e put può essere modificato da un altro thread. Dovrebbe essere:

 synchronized(map) { if(!map.containsKey("foo")) map.put("foo", "bar"); } 

O con un’implementazione di ConcurrentMap :

 map.putIfAbsent("foo", "bar"); 

Anche se probabilmente non è esattamente quello che stai chiedendo, il problema più frequente relativo alla concorrenza che ho riscontrato (probabilmente perché si presenta in un normale codice a thread singolo) è un

java.util.ConcurrentModificationException

causato da cose come:

 List list = new ArrayList(Arrays.asList("a", "b", "c")); for (String string : list) { list.remove(string); } 

Il bug più comune che vediamo dove lavoro è che i programmatori eseguono lunghe operazioni, come le chiamate al server, sull’EDT, bloccando la GUI per alcuni secondi e rendendo l’app non rispondente.

Dimenticando di aspettare () (o Condition.await ()) in un ciclo, controllando che la condizione di attesa sia effettivamente vera. Senza questo, si incontrano bug da spuri wait () wakeups. L’uso canonico dovrebbe essere:

  synchronized (obj) { while () { obj.wait(); } // do stuff based on condition being true } 

Un altro bug comune è la scarsa gestione delle eccezioni. Quando un thread in background genera un’eccezione, se non la gestisci correttamente, potresti non vedere affatto la traccia dello stack. O forse l’attività in background si interrompe e non si riavvia mai perché non hai gestito l’eccezione.

Può essere facile pensare che le raccolte sincronizzate ti garantiscano una protezione maggiore di quella che effettivamente fanno e dimentichi di tenere il blocco tra le chiamate. Ho visto questo errore alcune volte:

  List l = Collections.synchronizedList(new ArrayList()); String[] s = l.toArray(new String[l.size()]); 

Ad esempio, nella seconda riga sopra, i toArray() e size() sono entrambi sicuri per i thread, ma la size() viene valutata separatamente da toArray() e il blocco sull’Elenco non viene tenuto tra queste due chiamate.

Se esegui questo codice con un altro thread che rimuove contemporaneamente gli elementi dall’elenco, prima o poi ti ritroverai con una nuova String[] restituita che è più grande del necessario per contenere tutti gli elementi nell’elenco e ha valori null nella coda . È facile pensare che poiché le due chiamate di metodo all’Elenco avvengono in una singola riga di codice, questa è in qualche modo un’operazione atomica, ma non lo è.

Fino a quando non ho frequentato una lezione con Brian Goetz non mi sono reso conto che il getter non sincronizzato di un campo privato mutato attraverso un setter sincronizzato non ha mai la garanzia di restituire il valore aggiornato. Solo quando una variabile è protetta da un blocco sincronizzato su entrambe le letture E le scritture otterranno la garanzia dell’ultimo valore della variabile.

 public class SomeClass{ private Integer thing = 1; public synchronized void setThing(Integer thing) this.thing = thing; } /** * This may return 1 forever and ever no matter what is set * because the read is not synched */ public Integer getThing(){ return thing; } } 

Pensando di scrivere codice a thread singolo, ma usando statiche mutevoli (inclusi i singleton). Ovviamente saranno condivisi tra i thread. Questo accade sorprendentemente spesso.

Le chiamate al metodo arbitrario non devono essere effettuate da blocchi sincronizzati.

Dave Ray lo ha toccato nella sua prima risposta, e in effetti ho anche riscontrato un deadlock che ha a che fare anche con il richiamo di metodi sugli ascoltatori all’interno di un metodo sincronizzato. Penso che la lezione più generale sia che le chiamate al metodo non devono essere rese “in the wild” da un blocco sincronizzato – non si ha idea se la chiamata sarà di lunga durata, si verificherà un deadlock o qualsiasi altra cosa.

In questo caso, e generalmente in generale, la soluzione era di ridurre l’ambito del blocco sincronizzato per proteggere solo una sezione privata di codice critica.

Inoltre, dal momento che stavamo ora accedendo alla Raccolta di listener al di fuori di un blocco sincronizzato, lo abbiamo modificato per essere una raccolta copy-on-write. O avremmo potuto semplicemente fare una copia difensiva della Collezione. Essendo il punto, di solito ci sono alternative per accedere in sicurezza a una raccolta di oggetti sconosciuti.

Ho riscontrato un problema di concorrenza con Servlet, quando ci sono campi mutabili che verranno impostati da ogni richiesta. Ma c’è solo un’istanza di servlet per tutte le richieste, quindi questo ha funzionato perfettamente in un singolo ambiente utente, ma quando più di un utente ha richiesto il risultato imprevedibile della servlet.

 public class MyServlet implements Servlet{ private Object something; public void service(ServletRequest request, ServletResponse response) throws ServletException, IOException{ this.something = request.getAttribute("something"); doSomething(); } private void doSomething(){ this.something ... } } 

Il bug più recente relativo alla concorrenza in cui mi sono imbattuto era un object che nel suo costruttore creava un ExecutorService, ma quando l’object non era più referenziato, non aveva mai spento ExecutorService. Quindi, in un periodo di settimane, trapelarono migliaia di thread, causando infine il crash del sistema. (Tecnicamente, non si è bloccato, ma ha smesso di funzionare correttamente, continuando a funzionare.)

Tecnicamente, suppongo che questo non sia un problema di concorrenza, ma è un problema relativo all’uso delle librerie java.util.concurrency.

Il mio più grande problema è sempre stato il deadlock, soprattutto causato da ascoltatori che sono stati licenziati con una serratura bloccata. In questi casi, è davvero facile ottenere il blocco invertito tra due thread. Nel mio caso, tra una simulazione in esecuzione in un thread e una visualizzazione della simulazione eseguita nel thread dell’interfaccia utente.

MODIFICA: spostata la seconda parte per separare la risposta.

La sincronizzazione sbilanciata, in particolare contro Maps, sembra essere un problema abbastanza comune. Molte persone credono che la sincronizzazione su puts su una mappa (non una ConcurrentMap, ma una HashMap) e la non sincronizzazione su gets sia sufficiente. Questo tuttavia può portare ad un loop infinito durante re-hash.

Lo stesso problema (sincronizzazione parziale) può verificarsi ovunque tu abbia uno stato condiviso con letture e scritture comunque.

Non esattamente un bug, ma il peggior peccato è fornire una libreria che intendi usare da altre persone, ma non affermare quali classi / metodi sono thread-safe e quali devono essere chiamati solo da un singolo thread, ecc.

Altre persone dovrebbero fare uso delle annotazioni sulla concorrenza (ad esempio @ThreadSafe, @GuardedBy ecc.) Descritte nel libro di Goetz.

Avviare un thread all’interno del costruttore di una class è problematico. Se la class è estesa, il thread può essere avviato prima che il costruttore della sottoclass venga eseguito.

Classi mutabili in strutture di dati condivise

 Thread1: Person p = new Person("John"); sharedMap.put("Key", p); assert(p.getName().equals("John"); // sometimes passes, sometimes fails Thread2: Person p = sharedMap.get("Key"); p.setName("Alfonso"); 

Quando ciò accade, il codice è molto più complesso di questo esempio semplificato. Replicare, trovare e correggere l’errore è difficile. Forse potrebbe essere evitato se potessimo contrassegnare certe classi come strutture di dati immutabili e certe come se fossero solo oggetti immutabili.

La sincronizzazione su una stringa letterale o costante definita da una stringa letterale è (potenzialmente) un problema poiché la stringa letterale è internata e sarà condivisa da chiunque altro nella JVM utilizzando lo stesso letterale stringa. So che questo problema si è verificato nei server delle applicazioni e in altri scenari “contenitori”.

Esempio:

 private static final String SOMETHING = "foo"; synchronized(SOMETHING) { // } 

In questo caso, chiunque usi la stringa “foo” per bloccare sta condividendo lo stesso blocco.

Credo che in futuro il problema principale con Java sarà la (mancanza di) garanzie di visibilità per i costruttori. Ad esempio, se crei la seguente class

 class MyClass { public int a = 1; } 

e quindi basta leggere la proprietà di MyClass a da un altro thread, MyClass.a potrebbe essere 0 o 1, a seconda dell’implementazione e dell’umore di JavaVM. Oggi le possibilità di essere “1” sono molto alte. Ma sulle future macchine NUMA questo potrebbe essere diverso. Molte persone non ne sono consapevoli e ritengono di non aver bisogno di occuparsi del multithreading durante la fase di inizializzazione.

L’errore più stupido che faccio spesso è dimenticare di sincronizzare prima di chiamare notify () o wait () su un object.

Usando un “nuovo object ()” locale come mutex.

 synchronized (new Object()) { System.out.println("sdfs"); } 

Questo è inutile.

Un altro problema comune di “concorrenza” è l’uso del codice sincronizzato quando non è affatto necessario. Ad esempio vedo ancora programmatori che usano StringBuffer o anche java.util.Vector (come variabili locali del metodo).

Oggetti multipli che sono protetti da blocco ma sono comunemente accessibili in successione. Abbiamo incontrato un paio di casi in cui i blocchi sono stati ottenuti con codice diverso in diversi ordini, il che ha provocato un deadlock.

Non rendersi conto che this in una class interiore non è this della class esterna. Tipicamente in una class interna anonima che implementa Runnable . Il problema di root è che, poiché la sincronizzazione è parte di tutti gli Object , non esiste in effetti alcun controllo di tipo statico. L’ho visto almeno due volte su usenet, e compare anche in Brian Goetz’z Java Concurrency in Practice.

Le chiusure BGGA non ne soffrono perché non c’è this per la chiusura ( this riferimento alla class esterna). Se usi non oggetti come blocchi, si aggira questo problema e altri.

Utilizzo di un object globale come una variabile statica per il blocco.

Questo porta a prestazioni pessime a causa della contesa.

Honesly? Prima dell’avvento di java.util.concurrent , il problema più comune a cui mi sono imbattuto abitualmente era quello che chiamo “thread-thrashing”: applicazioni che usano thread per la concorrenza, ma ne generano troppi e finiscono per essere thrashing.