lapolopo ha scritto:
Come faccio a capire se un metodo ha bisogno di un Lock().
Mettiamola un po' in generale, in ambito multi-threading la "sincronizzazione" tra thread serve principalmente in uno scenario molto importante:
quando due (o più) thread devono accedere concorrentemente ad uno stato "mutabile" (in Java lo stato, cioè i dati, stanno principalmente negli oggetti, ovvero nelle variabili "di istanza" degli oggetti).
Se invece un oggetto non ha stato o ha stato
immutabile, se non ci sono altre questioni (che bisognerebbe precisare e indagare), allora di norma è intrinsecamente thread-safe e la sincronizzazione
non serve.
lapolopo ha scritto:
Nello specifico molte volte inserisco Lock dove nn ci vorrebbe.
Ho scritto questa classe di Prova provando a vedere se mettere o no il Lock nel metodo getincr() apportasse cambiamenti al mio output ma niente.
Innanzitutto un conto è usare il lock intrinseco degli oggetti (con la parola chiave synchronized), un altro conto è usare i lock "espliciti" forniti dal framework.
Tornando al tuo codice: la tua classe Counter contiene "stato" mutabile, modificabile da più thread. Quindi qui la sincronizzazione (locking) SERVE.
Ora, faccio solo una precisazione prima. I lock non servono solo a rendere un blocco di codice "atomico", garantendo la "mutua esclusione" tra thread. Servono anche ad un'altra cosa: a garantire la "visibilità" delle modifiche agli altri thread. Ci sarebbe un discorso un po' più lungo da fare ma prendilo così per ora. In sostanza: ANCHE il getincr() deve usare il lock!
Stai cercando di vedere se l'output cambia oppure no. Bene, ti propongo invece questo codice. Riprendo il tuo Counter (corretto nel getincr()):
class Counter {
private int counter;
private ReentrantLock lock = new ReentrantLock();
public void incr() {
lock.lock();
try {
counter++;
} finally {
lock.unlock();
}
}
public void decr() {
lock.lock();
try {
counter--;
} finally {
lock.unlock();
}
}
public int getincr() {
lock.lock();
try {
return counter;
} finally {
lock.unlock();
}
}
}
E poi ti propongo un main() più piccolo e semplificato:
import java.util.concurrent.locks.ReentrantLock;
public class ProvaCounter {
public static void main(String[] args) throws InterruptedException {
final Counter c = new Counter();
Thread[] threads = new Thread[20];
for (int i = 0; i < threads.length; i++) {
Runnable r = new Runnable() {
public void run() {
for (int n = 0; n < 10000; n++) {
c.incr();
}
}
};
threads[i] = new Thread(r);
threads[i].start();
}
// DEVE attendere la terminazione di TUTTI i thread PRIMA di poter stampare il counter!
for (Thread t : threads) {
t.join();
}
System.out.println("counter = " + c.getincr());
}
}
Ora, qui ci sono 20 thread e ciascuno incrementa di 10000. Quindi a rigor di logica il risultato complessivo dovrebbe essere 200000. Prova quante volte vuoi il codice, otterrai sempre
counter = 200000
Adesso invece
togli (o "commenta") tutti i lock() e unlock() in Counter. Poi riprova il ProvaCounter diciamo anche solo 10 volte e dimmi che output ottieni ...
Nota che nei thread NON ho messo alcun sleep(). C'è un motivo: uno sleep di 500ms come hai fatto tu rende più improbabile (ma non impossibile ovviamente) il fatto che due thread si "contendano" il lock, semplicemente perché stanno molto di più in sospensione piuttosto che "scocciarsi" a vicenda per acquisire il lock.
Tutto chiaro ora??