[Java] Problemino con i threads

[Groove89]

Sto cercando di creare un giochino in cui due giocatori si sfidano ed il loro scopo è cercare di finire il prima possibile un cesto di mele. Il primo che finisce vince.

I problemi sono 3:

1) Non mi piace il codice che ho scritto
2) Il codice non funziona alla perfezione, nel senso che delle volte segna come
vincitore anche quello che ha perso, e vorrei che una volta che uno dei
due giocatori ha finito il cesto, il programma termini.
3) Vorrei modificarlo in modo da usare wait() e notify().

Questo è il codice:

Cesto

package giocodellamela;
/*
 * Classe che rappresenta un cesto pieno di mele, che
 * può essere svuotato dai giocatori.
 */
public class Cesto {

	private int mele;
	private int meleMangiate;
	
	public Cesto(int mele) {
		this.mele = mele;
		this.meleMangiate = 0;
	}
	
	public int getMele() {
		return mele;
	}
	
	public int getMeleMangiate() {
		return meleMangiate;
	}
	
	public void setMele(int mele) {
		this.mele = mele;
	}
	
	public void mangiaMela() {
		if(!meleFinite()) {
			mele--;
		}
		meleMangiate++;
	}
	
	public boolean meleFinite() {
		return mele == 0;
	}
}

Giocatore

import java.util.Random;

/*
 * Classe che rappresenta un giocatore. 
 * Il suo scopo è mangiare tutte le mele presenti 
 * nel suo cesto prima degli altri giocatori.
 */
public class Giocatore implements Runnable {

	private String nome;
	private Cesto cesto;
	private Controllore controllore;
	private Random random = new Random();
	
	public Giocatore(String nome, Cesto cesto, Controllore c) {
		this.nome = nome;
		this.cesto = cesto;
		this.controllore = c;
	}
	
	public void run() {
		
		while(!cesto.meleFinite()) {
			cesto.mangiaMela();	
			stampa();
			attendi();
		}
		
		controllore.setVincitore(this.nome);
	}
	
	/*
	 * Simula una pausa random tra una mela mangiata ed un'altra
	 */
	private void attendi() {
		//Genero un'attesa tra 100 e 1000 millisecondi
		int j = 100;
		int n = 1000 - j;
		int attesa = random.nextInt(n) + j;
		try {
			Thread.sleep(attesa);
		} catch (InterruptedException e) {
			System.out.println("Qualcosa è andato storto...");
		}
	}
	
	public void stampa() {
		String s = "Il giocatore " + this.nome + " ha mangiato " + cesto.getMeleMangiate() + " mele" + "\n";
		System.out.println(s);
	}
}

Controllore

package giocodellamela;

public class Controllore {
	
	private String vincitore = null;
	
	public synchronized void setVincitore(String vincitore) {
		this.vincitore = vincitore;
	}
	
	public synchronized String getVincitore() {
		return vincitore;
	}
}

Gestore

package giocodellamela;

public class Gestore extends Thread {
	
	private static Controllore controllore;
	
	public Gestore() {
		controllore = new Controllore();
	}
	
	/* Controllo continuo della partita
	 * per verificare chi è il vincitore.
	 */
	public void run() {
		
		while(controllore.getVincitore() == null) {}
		
		System.out.println("Il vincitore è: " + controllore.getVincitore());
	}

	public static void main(String[]args) {
		
		Gestore gestore = new Gestore();
	
		Cesto primoCesto = new Cesto(20);
		Cesto secondoCesto = new Cesto(20);
		Giocatore andrea = new Giocatore("Andrea", primoCesto, controllore);
		Giocatore loris = new Giocatore("Loris", secondoCesto, controllore);
		
		Thread andreaThread = new Thread(andrea);
		Thread lorisThread = new Thread(loris);
		
		gestore.setPriority(MAX_PRIORITY);
		gestore.start();
		andreaThread.start();
		lorisThread.start();
	}
}

Vi ringrazio per qualsiasi tipo di aiuto

[*andre*]

Il codice non va perché nella zona sincronizzata non controlli se qualcuno ha già impostato il vincitore, quindi vincerà quello che ci mette di più (il perdente, anche se dipendendo dall'ordine dei thread lo stamperà bene o male).

Prova a controllare quella condizione.

Ad occhio, dovresti contare una mela mangiata solo se si sottrae dal contatore delle mele no?

Il Gestore sta effettuando un'attesa attiva sui thread. Prova ad usare il metodo .join().

Non ti fidare del .setPriority(..), dipende molto dall'implementazione della JVM e del sistema operativo.


Cos'ha che non ti piace il codice? Come lo vorresti migliorare?
Come avresti pensato di usare il wait-notify? Perché?

[dierre]

Ciao, sebbene sia anch'io poco esperto coi thread, ho provato a veder la cosa ed a risolverla.

Ho riscritto il tuo gioco ipotizzando appunto che ognuno avesse il suo cesto di mele da consumare.

Ho provato a vedere di correggere il tuo ma onestamente non mi veniva proprio perché il fatto che tu avessi il cestino all'interno del giocatore, cozzava con il requisito che hai espresso di voler fermare il gioco non appena il primo giocatore avesse finito.

Il cestino come lo hai messo tu purtroppo non permette (o comunque io non ci sono riuscito) di fermare il gioco quando il primo giocatore ha finito.

Non è possibile perché devi immaginarti come si tu avessi due cesti e poi due giocatori. Affinché il perdente si fermi deve esserci un arbitro che decreta la fine.

Questo requisito l'ho tradotto come una richiesta all'arbitro di poter mangiare.

Con la soluzione a cui ho pensato un tipico output è questo:

Peter Dinklage is missing 9 to remove
Jon Snow is missing 9 to remove
Peter Dinklage is missing 8 to remove
Jon Snow is missing 8 to remove
Peter Dinklage is missing 7 to remove
Jon Snow is missing 7 to remove
Jon Snow is missing 6 to remove
Peter Dinklage is missing 6 to remove
Jon Snow is missing 5 to remove
Peter Dinklage is missing 5 to remove
Peter Dinklage is missing 4 to remove
Peter Dinklage is missing 3 to remove
Jon Snow is missing 4 to remove
Peter Dinklage is missing 2 to remove
Peter Dinklage is missing 1 to remove
Jon Snow is missing 3 to remove
Peter Dinklage is missing 0 to remove
And the winner is Peter Dinklage

Peter Dinklage is missing 9 to remove
Jon Snow is missing 9 to remove
Peter Dinklage is missing 8 to remove
Jon Snow is missing 8 to remove
Jon Snow is missing 7 to remove
Peter Dinklage is missing 7 to remove
Jon Snow is missing 6 to remove
Jon Snow is missing 5 to remove
Peter Dinklage is missing 6 to remove
Jon Snow is missing 4 to remove
Jon Snow is missing 3 to remove
Jon Snow is missing 2 to remove
Peter Dinklage is missing 5 to remove
Peter Dinklage is missing 4 to remove
Jon Snow is missing 1 to remove
Peter Dinklage is missing 3 to remove
Jon Snow is missing 0 to remove
And the winner is Jon Snow

E questo è il sorgente:

class Player implements Runnable {

    String name;
    GameManager gameManager;

    Player(String name, GameManager gameManager) {
	this.name = name;
	this.gameManager = gameManager;
    }
    
    public String getName() {
	return name;
    }
    
    private void waitForSomeTime() {
	int j = 100;
	int n = 1000 - j;
	int attesa = new Random().nextInt(n) + j;
	try {
		Thread.sleep(attesa);
	} catch (InterruptedException e) {
		System.out.println("Qualcosa è andato storto...");
	}
    }

    @Override
    public void run() {
	while (gameManager.onGoing()) {
	    gameManager.removeApple(this);
	    waitForSomeTime();
	}
    }

    @Override
    public int hashCode() {
	final int prime = 31;
	int result = 1;
	result = prime * result + ((name == null) ? 0 : name.hashCode());
	return result;
    }

    @Override
    public boolean equals(Object obj) {
	if (this == obj)
	    return true;
	if (obj == null)
	    return false;
	if (getClass() != obj.getClass())
	    return false;
	Player other = (Player) obj;
	if (name == null) {
	    if (other.name != null)
		return false;
	} else if (!name.equals(other.name))
	    return false;
	return true;
    }

}

class Bucket {
    private int things;

    public Bucket(int things) {
	this.things = things;
    }

    public boolean isEmpty() {
	return things == 0;
    }

    public void poll() {
	things--;
    }

    public int getThings() {
	return things;
    }
}

class WinnerNotifer implements Runnable {

    volatile Player winner = null;

    public void setWinner(Player winner) {
	this.winner = winner;
	wakeUp();
    }
    
    private synchronized void wakeUp() {
	notifyAll();
    }

    public void waitForWinner() {
	while (winner == null)
	    synchronized (this) {
		try {
		    wait();
		} catch (InterruptedException e) {
		    e.printStackTrace();
		}
	    }
	System.out.println("And the winner is " + winner.getName());
    }

    @Override
    public void run() {
	waitForWinner();
    }
}
class GameManager {

    volatile boolean onGoing = true;

    Map<Player, Bucket> contestant;

    private WinnerNotifer winnerNotifier;

    public GameManager(WinnerNotifer winnerNotifier) {
	contestant = new HashMap<Player, Bucket>();
	this.winnerNotifier = winnerNotifier;
    }

    public void addContestant(Player player, Bucket bucket) {
	contestant.put(player, bucket);
    }

    public boolean onGoing() {
	return onGoing;
    }

    public synchronized void removeApple(Player player) {
	if (onGoing) {
	    Bucket currentBucket = contestant.get(player);
	    currentBucket.poll();
	    System.out.println(player.getName() + " is missing "
		    + currentBucket.getThings() + " to remove");
	    if (currentBucket.isEmpty()) {
		onGoing = false;
		winnerNotifier.setWinner(player);
	    }
	}
    }

}

public class Game {
    public static void main(String[] args) {
	
	WinnerNotifer winnerNotifer = new WinnerNotifer();
	
	GameManager gameManager = new GameManager(winnerNotifer);
	
	Player player1 = new Player("Jon Snow", gameManager);
	Player player2 = new Player("Peter Dinklage", gameManager);
	
	gameManager.addContestant(player1, new Bucket(10));
	gameManager.addContestant(player2, new Bucket(10));
	
	new Thread(winnerNotifer).start();
	new Thread(player2).start();
	new Thread(player1).start();
    }
}

Sono qui sei hai domande
Tutti i consigli sono ben accetti.

[Groove89]

Grazie delle risposte

Prima di ragionare su quello che avete fatto e detto, volevo dire che nel frattempo ero arrivato ad una soluzione temporanea. Nel senso che ora il codice sembra funzionare, ma devo ancora migliorarlo con l'implementazione di
wait() e notify(). Perché? Perché il Gestore effetta un polling tramite while per capire se c'è un vincitore, il che è molto dispendioso e "brutto".
Credo che il codice possa essere migliorato. Non mi piace che il Gestore stesso sia un Thread, e da quel che so è sconsigliato estendere la classe Thread. Non mi piace la variabile statica Controllore in Gestore. Comunque, allo stato attuale il codice è questo:

Cesto

Uguale a prima

Controllore

package giocodellamela;

public class Controllore {
	
	private String vincitore;
	private boolean flag;
	
	public Controllore() {
		 this.vincitore = null;
		 flag = false;
	}
	
	public synchronized void setNomeVincitore(String vincitore) {
		this.vincitore = vincitore;
	}
	
	public synchronized String getVincitore() {
		return vincitore;
	}
	
	public synchronized void setVincitore() {
		flag = true;
	}
	
	public synchronized boolean thereIsVincitore() {
		return flag == true;
	}
}

Giocatore

package giocodellamela;

import java.util.Random;

/*
 * Classe che rappresenta un giocatore. 
 * Il suo scopo è mangiare tutte le mele presenti 
 * nel suo cesto prima degli altri giocatori.
 */
public class Giocatore implements Runnable {

	private String nome;
	private Cesto cesto;
	private Controllore controllore;
	private Random random = new Random();
	
	public Giocatore(String nome, Cesto cesto, Controllore c) {
		this.nome = nome;
		this.cesto = cesto;
		this.controllore = c;
	}
	
	public void run() {
		
		while(!cesto.meleFinite()) {
			if(!controllore.thereIsVincitore()) {
				cesto.mangiaMela();	
				stampa();
				attendi();
			}
			else {
				return;
			}
		}
		
		if(!controllore.thereIsVincitore()) {
			controllore.setVincitore();
			controllore.setNomeVincitore(this.nome);
		}
		else {
			return;
		}
	}
	
	/*
	 * Simula una pausa random tra una mela mangiata ed un'altra
	 */
	private void attendi() {
		//Genero un'attesa tra 100 e 1000 millisecondi
		int j = 100;
		int n = 1000 - j;
		int attesa = random.nextInt(n) + j;
		try {
			Thread.sleep(attesa);
		} catch (InterruptedException e) {
			System.out.println("Qualcosa è andato storto...");
		}
	}
	
	public void stampa() {
		String s = "Il giocatore " + this.nome + " ha mangiato " + cesto.getMeleMangiate() + " mele" + "\n";
		System.out.println(s);
	}
}

Gestore

package giocodellamela;

public class Gestore extends Thread {
	
	private static Controllore controllore;
	
	public Gestore() {
		controllore = new Controllore();
	}
	
	/* Controllo continuo della partita
	 * per verificare chi è il vincitore.
	 */
	public void run() {
		
		while(controllore.getVincitore() == null) {}
		
		System.out.println("Il vincitore è: " + controllore.getVincitore() + "\n");
	}

	public static void main(String[]args) {
		
		Gestore gestore = new Gestore();
	
		Cesto primoCesto = new Cesto(20);
		Cesto secondoCesto = new Cesto(20);
		Giocatore andrea = new Giocatore("Andrea", primoCesto, controllore);
		Giocatore loris = new Giocatore("Loris", secondoCesto, controllore);
		
		Thread andreaThread = new Thread(andrea);
		Thread lorisThread = new Thread(loris);
		
		gestore.start();
		andreaThread.start();
		lorisThread.start();
	}
}

No cavolo, non va uguale. Può capitare che il giocatore che ha perso continui a mangiare qualche mela prima di fermarsi -.- Dierre, sto provando a studiare il tuo codice...

[dierre]

Originariamente inviato da Groove89

[...]

No cavolo, non va uguale. Può capitare che il giocatore che ha perso continui a mangiare qualche mela prima di fermarsi -.- Dierre, sto provando a studiare il tuo codice...

E' normale, purtroppo non stai facendo un'operazione atomica. L'altro giocatore nel frattempo continua a fare quello che vuole fra i due check. Non puoi prescindere da un'area di mutua esclusione secondo me.

[Groove89]

Ok, il tuo codice mi sembra perfetto. Lo assumo come "base" per programmi di questo tipo

Ora sai che faccio, mi invento un altro gioco e lo implemento. Pensavo ad esempio di fare un gioco in cui due giocatori devono mettersi a cercare all'interno di un contenitore un oggetto speciale, tra tanti normali. Il primo che lo trova vince. In questo caso i giocatori svolgono un azione all'interno dello stesso oggetto che condividono totalmente..

Ps: vediamo se ho capito la logica del tuo codice. Finché il gioco è attivo (ci sono ancora mele) i giocatori mangiano le mele. Nel frattempo winnerNotifer controlla di continuo se c'è un vincitore. Non ho capito bene lo scopo della synchronized(this). Comunque, se un giocatore vince (questo viene controllato da GameManager), esso setta il vincitore in winnerNotifer che si sveglia tramite la notifyAll (non era sufficiente una notify?), nel run esce dal wait e stampa il vincitore.

[dierre]

Originariamente inviato da Groove89

Ok, il tuo codice mi sembra perfetto. Lo assumo come "base" per programmi di questo tipo

Ora sai che faccio, mi invento un altro gioco e lo implemento. Pensavo ad esempio di fare un gioco in cui due giocatori devono mettersi a cercare all'interno di un contenitore un oggetto speciale, tra tanti normali. Il primo che lo trova vince. In questo caso i giocatori svolgono un azione all'interno dello stesso oggetto che condividono totalmente..

Ps: vediamo se ho capito la logica del tuo codice. Finché il gioco è attivo (ci sono ancora mele) i giocatori mangiano le mele. Nel frattempo winnerNotifer controlla di continuo se c'è un vincitore. Non ho capito bene lo scopo della synchronized(this). Comunque, se un giocatore vince (questo viene controllato da GameManager), esso setta il vincitore in winnerNotifer che si sveglia tramite la notifyAll (non era sufficiente una notify?), nel run esce dal wait e stampa il vincitore.

Più che inventarti un gioco, che è una cosa ottima da fare dopo, guardati i problemi dei lettori/scrittori, 5 filosofi etc... Insomma i problemi della concorrenza classica.

Non prendere il mio codice come esempio, anch'io sto imparando.

No, non controlla di continuo. Il while viene usato come guarded block per proteggere il metodo da un wake up accidentale da parte della jvm. In questo modo dovesse succedere, ritorna in wait. La wait sospende il thread e rilascia il lock, eviti la busy waiting che tu facevi prima.
Il syncronized(this) è obbligatorio con wait o notify/notifyAll perché non puoi usare quei metodi se prima non hai il lock dell'oggetto su cui vuoi chiamare il metodo (in questo caso this). Prova a toglierlo e guarda che succede a runtime.
Non è una best practice usare notify. Va usato notifyAll ed eventualmente gestire il wake up attraverso una guarded block di tutti i thread interessati.
Ti è sfuggito, credo, l'uso della keyword volatile. Questa si usa affinché il valore della variabile letto non sia stale in caso di multithreading, cioè non aggiornato all'ultimo valore effettivo.

Ti consiglio di leggerti il Java Tutorial ufficiale sulla concorrenza.

[starfred]

Ciao, volevo fare delle piccole osservazioni, il codice di dierre mi sembra corretto tuttavia perde la forza del parallelismo dei thread poiché il metodo removeApple è sync quindi solo un thread alla volta può accedervi quindi non hai la condizione in cui i 2 thread mangiano contemporaneamente una mela dal proprio cestino.

Io personalmente, cercando di cambiare il meno possibile il codice originiale, modificherei la funzione setNomeVincitore aggiungendo un controllo del tipo:

...
if (flag == false)
{
setta nome vincitore;
setta flag = true
}
return;
...

Mentre invece cambierei la funzione mangiamela aggiungendo come parametro il controllore ed il nome quindi una cosa del tipo:

public void mangiaMela(String nome, Controllore C)
{
...
Decrementa il numero di mele
se il numero di mele == 0 allora C.SetNomeVincitore (nome)
...
}

Ho scritto in pseudo codice per cercare di essere il più chiaro possibile, come ti sarà venuto in mente ora è anche possibile snellire parecchio la run del Giocatore in quanto molti controlli non servono più.
Ciao.

[Groove89]

Allora, ho capito il codice di dierre, a grandi linee almeno. Per entrare perfettamente nell'ottica ho bisogno ancora di tempo

Comunque, siccome mi dispiace buttare il mio, sto cercando di modificarlo prendendo spunto da quello di dierre, ma continua a dare lo stesso problema.

Ecco il codice:

Giocatore

package giocodellamela;

import java.util.Random;

/*
 * Classe che rappresenta un giocatore. 
 * Il suo scopo è mangiare tutte le mele presenti 
 * nel suo cesto prima degli altri giocatori.
 */
public class Giocatore implements Runnable {

	private String nome;
	private Cesto cesto;
	private Controllore controllore;
	private Random random = new Random();
	
	public Giocatore(String nome, Cesto cesto, Controllore c) {
		this.nome = nome;
		this.cesto = cesto;
		this.controllore = c;
	}
	
	public void run() {
		
		while(!controllore.thereIsVincitore()) {
			if(!cesto.isEmpty()) {
				cesto.pull();
				System.out.println("Il giocatore " + nome + " ha mangiato " + cesto.getMeleMangiate() + " mele.");
				attendi();
			}
			else {
				controllore.setVincitore(nome);
			}
		}
	}
	
	/*
	 * Simula una pausa random tra una mela mangiata ed un'altra
	 */
	private void attendi() {
		//Genero un'attesa tra 100 e 1000 millisecondi
		int j = 100;
		int n = 1000 - j;
		int attesa = random.nextInt(n) + j;
		try {
			Thread.sleep(attesa);
		} catch (InterruptedException e) {
			System.out.println("Qualcosa è andato storto...");
		}
	}
	
	public void stampa() {
		String s = "Il giocatore " + this.nome + " ha mangiato " + cesto.getMeleMangiate() + " mele" + "\n";
		System.out.println(s);
	}
}

Cesto

package giocodellamela;
/*
 * Classe che rappresenta un cesto pieno di mele, che
 * può essere svuotato dai giocatori.
 */
public class Cesto {

	private int mele;
	private int meleMangiate;
	
	public Cesto(int mele) {
		this.mele = mele;
		this.meleMangiate = 0;
	}
	
	public int getMele() {
		return mele;
	}
	
	public int getMeleMangiate() {
		return meleMangiate;
	}
	
	public void setMele(int mele) {
		this.mele = mele;
	}
	
	public void pull() {
		mele--;
		meleMangiate++;
	}
	
	public boolean isEmpty() {
		return mele == 0;
	}
}

Controllore

package giocodellamela;

public class Controllore implements Runnable {
	
	private String vincitore;
	private boolean flag;
	
	public Controllore() {
		 this.vincitore = null;
		 flag = false;
	}
	
	public void run() {
		attendiVincitore();
	}
	
	public synchronized String getVincitore() {
		if(vincitore != null) {
			return vincitore;
		}
		else return null;
	}
	
	public synchronized void setVincitore(String nome) {
		vincitore = nome;
		flag = true;
		wakeUp();
	}
	
	public synchronized boolean thereIsVincitore() {
		return flag == true;
	}
	
	public void attendiVincitore() {
		synchronized(this) {
			while(flag == false) {
				try {
					wait();
				} catch (InterruptedException e) {
					e.printStackTrace();
				}
			}
		}
		System.out.println("Il vincitore è " + this.vincitore);
	}
	
	public void wakeUp() {
		notifyAll();
	}
}

Gestore

package giocodellamela;

public class Gestore {
	
	private static Controllore controllore;
	
	public Gestore() {
		controllore = new Controllore();
	}
	
	public static void main(String[]args) {
		
		Gestore gestore = new Gestore();
	
		Cesto primoCesto = new Cesto(20);
		Cesto secondoCesto = new Cesto(20);
		Giocatore andrea = new Giocatore("Andrea", primoCesto, controllore);
		Giocatore loris = new Giocatore("Loris", secondoCesto, controllore);
		
		Thread andreaThread = new Thread(andrea);
		Thread lorisThread = new Thread(loris);
		Thread control = new Thread(controllore);
		
		control.start();
		andreaThread.start();
		lorisThread.start();
	}
}

[dierre]

Originariamente inviato da starfred

Ciao, volevo fare delle piccole osservazioni, il codice di dierre mi sembra corretto tuttavia perde la forza del parallelismo dei thread poiché il metodo removeApple è sync quindi solo un thread alla volta può accedervi quindi non hai la condizione in cui i 2 thread mangiano contemporaneamente una mela dal proprio cestino.
[...]

Ciao, ho seguito il tuo input e concordo. Ho modificato il mio codice per spostare il consumo di mele nel giocatore. Fammi sapere cosa ne pensi.

class Player implements Runnable
{

  String      name;
  GameManager gameManager;
  Bucket      bucket;

  Player (String name, GameManager gameManager, Bucket bucket)
  {
    this.name = name;
    this.gameManager = gameManager;
    this.bucket = bucket;
  }

  public String getName ()
  {
    return name;
  }

  private void waitForSomeTime ()
  {
    int j = 100;
    int n = 1000 - j;
    int attesa = new Random().nextInt(n) + j;
    try
    {
      Thread.sleep(attesa);
    }
    catch (InterruptedException e)
    {
      System.out.println("Qualcosa è andato storto...");
    }
  }

  @Override
  public void run ()
  {
    while (gameManager.onGoing())
    {
      bucket.poll();
      System.out.println(getName() + " is missing " + bucket.getThings() + " to remove");
      if(bucket.isEmpty()) {
        gameManager.reclaimWin(this);
        return;
      }
      waitForSomeTime();        
    }
  }

  @Override
  public int hashCode ()
  {
    final int prime = 31;
    int result = 1;
    result = prime * result + ((name == null) ? 0 : name.hashCode());
    return result;
  }

  @Override
  public boolean equals (Object obj)
  {
    if (this == obj) return true;
    if (obj == null) return false;
    if (getClass() != obj.getClass()) return false;
    Player other = (Player) obj;
    if (name == null)
    {
      if (other.name != null) return false;
    }
    else if (!name.equals(other.name)) return false;
    return true;
  }

}

class Bucket
{
  private int things;

  public Bucket (int things)
  {
    this.things = things;
  }

  public boolean isEmpty ()
  {
    return things == 0;
  }

  public void poll ()
  {
    things--;
  }

  public int getThings ()
  {
    return things;
  }
}

class WinnerNotifer implements Runnable
{

  volatile Player winner = null;

  public void setWinner (Player winner)
  {
    this.winner = winner;
    wakeUp();
  }

  private synchronized void wakeUp ()
  {
    notifyAll();
  }

  public void waitForWinner ()
  {
    while (winner == null)
      synchronized (this)
      {
        try
        {
          wait();
        }
        catch (InterruptedException e)
        {
          e.printStackTrace();
        }
      }
    System.out.println("And the winner is " + winner.getName());
  }

  @Override
  public void run ()
  {
    waitForWinner();
  }
}

class GameManager
{

  volatile boolean      onGoing = true;

  private WinnerNotifer winnerNotifier;

  public GameManager (WinnerNotifer winnerNotifier)
  {
    this.winnerNotifier = winnerNotifier;
  }

  public syncronized void reclaimWin (Player player)
  {
    if (onGoing) {
      onGoing = false;
      winnerNotifier.setWinner(player);
    }
  }

  public boolean onGoing ()
  {
    return onGoing;
  }

}

public class Game
{
  public static void main (String[] args)
  {
    WinnerNotifer winnerNotifer = new WinnerNotifer();
    GameManager gameManager = new GameManager(winnerNotifer);
    Player player1 = new Player("Jon Snow", gameManager, new Bucket(10));
    Player player2 = new Player("Peter Dinklage", gameManager, new Bucket(10));
    new Thread(winnerNotifer).start();
    new Thread(player2).start();
    new Thread(player1).start();
  }
}

Questa versione funziona per due motivi:

Il ciclo di eat viene fatto sul controllo della situazione della partita.
Il giocatore prima fa tutto quello che deve fare per mangare e poi dire all'arbitro che è il vincitore e dopo aspetta.
Mettendo l'attesa prima del reclaim del vincitore purtroppo può capitare che un altro sia entrato e nel frattempo abbia mangiato.

Non sono soddisfatto della cosa, il punto è che se la cosa da consumare è personale, comunque ci deve essere una gestione dei turni per mangiare sennò è ovvio che posso fare qualcosa anche dopo. La soluzione precedente gestiva questa problematica e soprattutto l'ordine di entrata era comunque regolato su prenotazione del lock condiviso.

Il concetto è che non è vero parallelismo. I cestini sono indipendenti finché non c'è un vincitore.

[Groove89]

Ma nella mia ultima versione, come mai appena c'è un vincitore il tizio che perde può continuare a mangiare?

[starfred]

Attenzione: il metodo public boolean onGoing () deve essere sync poiché si va a fare una lettura di un dato (onGoing) che potrebbe essere sotto scrittura dal metodo reclaimWin.

Per il discorso del fatto che potrebbe mangiare la mela è logicamente corretto dal punto di vista del parallelismo.
Immaginatevi i due giocatori che prima di mangiare guardano se una lampadina è accesa per sapere se qualcuno ha vinto, dopo di che abbassano la testa e mangiano la mela a questo punto se hanno finito le mele premono il pulsante della vittoria che accende la lampadina. Quindi se nel momento in cui abbasso la testa l'altro giocatore ha vinto io non lo posso sapere finché non la rialzo.

Ci sono diverse tecniche per interrompere l'altro giocatore, per esempio l'arbitro potrebbe invocare immediatamente la stop del thread perdente ma sinceramente come soluzione non si usa più da qualche anno.

Dal mio punto di vista essendo l'operazione "mangia la mela" poco onerosa dal punto di vista computazionale lo spreco di risorse è minimo.

Se invece l'operazione fosse molto onerosa tipo "scrivi 1GB su disco" allora dividerei questa operazione in una macchina a stati con diversi check sulla condizione... etc. etc.

Sto divagando troppo

[Groove89]

Capisco il tuo discorso,ma per quanto riguarda il mio codice vorrei che si comportasse in quel modo anche per esercizio. Non riesco bene a capire perché il giocatore che perde non si ferma immediatamente -.-

[dierre]

Originariamente inviato da Groove89

Capisco il tuo discorso,ma per quanto riguarda il mio codice vorrei che si comportasse in quel modo anche per esercizio. Non riesco bene a capire perché il giocatore che perde non si ferma immediatamente -.-

Perché i tuoi check non seguono l'ordine che tu pensi seguano

	public void run() {
		
		while(!controllore.thereIsVincitore()) {
			if(!cesto.isEmpty()) {
				cesto.pull();
				System.out.println("Il giocatore " + nome + " ha mangiato " + cesto.getMeleMangiate() + " mele.");
				attendi();
			}
			else {
				controllore.setVincitore(nome);
			}
		}
	}

Non ci sono operazioni espresse in maniera atomica. Tu stai assumendo che quando uno dei due giocatori è in fase di settaggio vincitore, l'altro si trova ad attendere su thereIsVincitore. Non è così ed inoltre non è garantito l'ordine di esecuzione in 2 thread diversi.

L'unico modo che hai per avere l'assoluta certezza che un giocatore non mangi quando l'altro ha vinto è che ci sia una zona condivisa a cui accedono entrambi in mutua esclusione.

@starfred: è un assegnamento su variabile volatile, non credo ci sia lettura di dato stale.

Per il discorso del fatto che potrebbe mangiare la mela è logicamente corretto dal punto di vista del parallelismo.
Immaginatevi i due giocatori che prima di mangiare guardano se una lampadina è accesa per sapere se qualcuno ha vinto, dopo di che abbassano la testa e mangiano la mela a questo punto se hanno finito le mele premono il pulsante della vittoria che accende la lampadina. Quindi se nel momento in cui abbasso la testa l'altro giocatore ha vinto io non lo posso sapere finché non la rialzo.

Ci sono diverse tecniche per interrompere l'altro giocatore, per esempio l'arbitro potrebbe invocare immediatamente la stop del thread perdente ma sinceramente come soluzione non si usa più da qualche anno.

Perdonami ma non è molto differente da quello che sto dicendo io. Se vuoi bloccare la mangiata ci deve essere una terza parte che lo segnali. Non vedo come si possa trascendere dal farli mangiare in maniera controllata (è un requisito chiesto dal thread starter lo stop immediato).

[Groove89]

Originariamente inviato da dierre

L'unico modo che hai per avere l'assoluta certezza che un giocatore non mangi quando l'altro ha vinto è che ci sia una zona condivisa a cui accedono entrambi in mutua esclusione.

E come si fa? Pensavo che per come è impostata la classe Controllore fosse sufficiente..

Forse sono arrivato alla soluzione ufficiale..

Cesto

package giocodellamela;
/*
 * Classe che rappresenta un cesto pieno di mele, che
 * può essere svuotato dai giocatori.
 */
public class Cesto {


    private int mele;
    
    public Cesto(int mele) {
        this.mele = mele;
    }
    
    public boolean mangiaMela() {
        if (mele < 1)
            return false;
        mele--;
        return true;
    }
    
    public boolean isEmpty() {
        return mele == 0;
    }
}

Giocatore

package giocodellamela;


import java.util.Random;


/*
 * Classe che rappresenta un giocatore. 
 * Il suo scopo è mangiare tutte le mele presenti 
 * nel suo cesto prima degli altri giocatori.
 */
public class Giocatore implements Runnable {


    private String nome;
    private Controllore controllore;
    private Random random = new Random();
    
    public Giocatore(String nome) {
        this.nome = nome;
    }
    
    public void setControllore(Controllore c) {
        this.controllore = c;
    }
    
    public void run() {
        
        while(controllore.mangiaMela(this)) {
            System.out.println(this + ": Mangio una mela!");
            attendi();
        }    
    }
    
    /*
     * Simula una pausa random tra una mela mangiata ed un'altra
     */
    private void attendi() {
        //Genero un'attesa tra 100 e 1000 millisecondi
        int j = 100;
        int n = 1000 - j;
        int attesa = random.nextInt(n) + j;
        try {
            Thread.sleep(attesa);
        } catch (InterruptedException e) {
            System.out.println("Qualcosa è andato storto...");
        }
    }
    
    public String toString() {
        return nome;
    }
}

Controllore

package giocodellamela;


public class Controllore {


    private Giocatore giocatoreA;
    private Giocatore giocatoreB;
    private Cesto cestoA;
    private Cesto cestoB;
    private int meleA;
    private int meleB;


    public Controllore(Giocatore gA, Giocatore gB, Cesto cA, Cesto cB) {
        this.giocatoreA = gA;
        this.giocatoreB = gB;
        this.cestoA = cA;
        this.cestoB = cB;
        meleA = 0;
        meleB = 0;
    }


    public synchronized boolean mangiaMela(Giocatore g) {


        if (giocatoreA.equals(g)) {
            if (cestoA.mangiaMela() && !cestoB.isEmpty()) {
                meleA++;
            } else
                return false;
        }


        if (giocatoreB.equals(g)) {
            if (cestoB.mangiaMela() && !cestoA.isEmpty()) {
                meleB++;
            } else
                return false;
        }


        return true;


    }


    public String getVincitore() {
        if (meleA == meleB)
            return giocatoreA + " e " + giocatoreB + " pareggiano con " + meleA
                    + " mele a testa!";
        if (meleA > meleB)
            return giocatoreA + " vince " + meleA + " a " + meleB;
        else
            return giocatoreB + " vince " + meleB + " a " + meleA;
    }
}

Gestore

package giocodellamela;


public class Gestore {
    
    public static void main(String[]args) throws InterruptedException {
        
        Gestore gestore = new Gestore();
    
        Cesto primoCesto = new Cesto(20);
        Cesto secondoCesto = new Cesto(20);
        
        Giocatore andrea = new Giocatore("Andrea");
        Giocatore loris = new Giocatore("Loris");
        
        Controllore controllore = new Controllore(andrea,loris,primoCesto,secondoCesto);
        
        andrea.setControllore(controllore);
        loris.setControllore(controllore);
        
        Thread andreaThread = new Thread(andrea);
        Thread lorisThread = new Thread(loris);
        
        andreaThread.start();
        lorisThread.start();
        
        andreaThread.join();
        andreaThread.join();
        
        System.out.println(controllore.getVincitore());
    }
}

[dierre]

Che poi è la soluzione che ho proposto io come prima

Occhio che c'è del codice duplicato negli if. Di solito si risolve rifattorizzando col polimorfismo.

[Groove89]

Non ho capito.

[dierre]

Qui c'è del codice duplicato.

        
if (giocatoreA.equals(g)) {
            if (cestoA.mangiaMela() && !cestoB.isEmpty()) {
                meleA++;
            } else
                return false;
        }


        if (giocatoreB.equals(g)) {
            if (cestoB.mangiaMela() && !cestoA.isEmpty()) {
                meleB++;
            } else
                return false;
        }

cambiano solo le variabili ma l'algoritmo è lo stesso in entrambi i casi. Dovesse cambiare la politica sulle mele, dovresti cambiare in due punti invece che in uno.

[starfred]

Originariamente inviato da dierre

@starfred: è un assegnamento su variabile volatile, non credo ci sia lettura di dato stale.

Il codice deve essere concettualmente giusto, il fatto che leggi una variabile che è potenzialmente sotto scrittura è errato.
Il fatto che sia in memoria volatile non garantisce la correttezza, solo in alcuni casi è corretto, per esempio quando in ASM esegui un'operazione tramite una singola istruzione.

Immagina se onGoing fosse un array di 2 elementi e supponi che l'operazione di scrittura esegua

onGoing[0]=0;
sleep(10)
onGoing[1]=1;

Se leggi nel momento in cui lo scrittore è in sleep(10) ecco fatto che sei in uno stato inconsistente.

[dierre]

Originariamente inviato da starfred

Il codice deve essere concettualmente giusto, il fatto che leggi una variabile che è potenzialmente sotto scrittura è errato.
Il fatto che sia in memoria volatile non garantisce la correttezza, solo in alcuni casi è corretto, per esempio quando in ASM esegui un'operazione tramite una singola istruzione.

Immagina se onGoing fosse un array di 2 elementi e supponi che l'operazione di scrittura esegua

onGoing[0]=0;
sleep(10)
onGoing[1]=1;

Se leggi nel momento in cui lo scrittore è in sleep(10) ecco fatto che sei in uno stato inconsistente.

Sì, ho capito il tuo ragionamento. Effettivamente non è una set secca della variabile. Correggerò.