[JAVA] Polling su file con thread

[GByTe87]

Buongiorno a tutti.

Stavo giocando con i thread in java per realizzare un semplice poller che ad intervalli regolari va a verificare se un certo file è presente e, in caso affermativo, lo processa (un banalissimo producer-consumer).

Il tutto funziona, ma ho un solo problema... temo che una volta processato il file il thread consumer rimanga in esecuzione (per dire, nella vista Debug di Eclipse non mi appare [Terminated]).. ho iniziato da poco a giocherellare con i thread e non escludo grosse cazzate.

Per il producer ho usato uno ScheduledExecutorService in modo da poter eseguire il check ogni n secondi.

Ecco il codice:

TestClass.java

import java.io.BufferedInputStream;
import java.io.DataInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;

public class TestClass {

	boolean isFilePresent = false;
	String filename;
	File xmlToRead = null;

	public TestClass(String filename) {
		this.filename = filename;
	}

	synchronized void checkFilePresence() {
		System.out.println("[checkFilePresence] IN");

		if (isFilePresent) {
			System.out.println("[checkFilePresence] File già letto.");
			return;
		}

		xmlToRead = new File(filename);

		if (xmlToRead.exists() == false) {
			System.out.println("[checkFilePresence] File non ancora presente..");
			return;
		}

		notify();
		System.out.println("[checkFilePresence] File trovato!");
	}

	synchronized void printFileContent(Producer producer) {
		System.err.println("[printFileContent] IN");

		if (!isFilePresent)
			try {
				wait();
			} catch (InterruptedException exc) {
				System.out.println("Caught InterruptedException in printFileContent");
			}

		// Processo file

		isFilePresent = false;
		producer.stop();

		System.err.println("[printFileContent] Out");
	}
}

Producer.java

import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

public class Producer {
	private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
	private ScheduledFuture<?> checkerHandle = null;

	public Producer(final TestClass test) {

		final Runnable checker = new Runnable() {

			@Override
			public void run() {
				test.checkFilePresence();
			}
		};

		checkerHandle = scheduler.scheduleAtFixedRate(checker, 1, 2, TimeUnit.SECONDS);
		scheduler.schedule(new Runnable() {

			@Override
			public void run() {
				checkerHandle.cancel(true);
			}
		}, 60, TimeUnit.SECONDS);
	}

	public void stop() {
		checkerHandle.cancel(true);
	}
}

Consumer.java

public class Consumer implements Runnable {

	private TestClass test;
	private Producer producer;

	public Consumer(TestClass test, Producer producer) {
		this.test = test;
		this.producer = producer;
		Thread thread = new Thread(this, "Consumer");
		thread.start();
	}

	public void run() {
		test.printFileContent(producer);

	}
}

Main.java

public class Main {

	public static void main(String[] args) {
		
		TestClass test = new TestClass("/home/gibbo/bla.log");
		Producer producer = new Producer(test);
		new Consumer(test, producer);
	}
}

Vi ringrazio

[banryu79]

Analizzando il codice che hai postato e il sintomo che rilevi (consumer non "muore") ipotizzo che si verifichi un deadlock.
In particolare quando consumer ottiene il lock sull'istanza di TestClass e isFilePresent vale false: consumer va in wait() durante la chiamata a printFileContent e non rilascia il lock sull'istanza.
Quindi il producer schedulato rimarrà in attesa di poter acquisire il lock chiamando il metodo checkFilePresence sull'istanza di TestClass, lock che non verrà liberato perchè posseduto da consumer che a sua volta attende che producer lo risvegli dalla wait().

[starfred]

Ho dato un'occhiata un po' rapida e mi sembra di notare che c'è la possibilità che il main termini prima dei thread. Non vedo da nessuna parte un join(). Comunque è un codice molto molto molto contorto, non so se per scelta tua o per specifiche.
Generalmente un thread viene implementato in una classe
esempio:

public class Producer extends Thread {
producer(){...}
run(){...}
}

Gli oggetti che devono essere usati concorrentemente non dovrebbero modificare i thread direttamente, ma dovrebbero essere i thread a modificarsi in base all'oggetto.
Tra l'altro mi sembra di capire il consumatore utilizzi la checkerHandle.cancel(true); per far terminare il loop del produttore; come prima facendo così aumenti di molto la probabilità di starvation.
Comunque non sto dicendo che sia sbagliato, solo che secondo me aumenti di molto la complessità e rischi di commettere parecchi errori.

ciao

[GByTe87]

Originariamente inviato da banryu79

Analizzando il codice che hai postato e il sintomo che rilevi (consumer non "muore") ipotizzo che si verifichi un deadlock.
In particolare quando consumer ottiene il lock sull'istanza di TestClass e isFilePresent vale false: consumer va in wait() durante la chiamata a printFileContent e non rilascia il lock sull'istanza.
Quindi il producer schedulato rimarrà in attesa di poter acquisire il lock chiamando il metodo checkFilePresence sull'istanza di TestClass, lock che non verrà liberato perchè posseduto da consumer che a sua volta attende che producer lo risvegli dalla wait().

Mmm, non capisco come il producer possa rimanere in attesa del consumer, non essendoci alcuna wait() in checkFilePresence.
(Può essere che non abbia afferrato chiaramente il funzionamento di wait() e notify(). Vado a ripassarli per sicurezza. )

Originariamente inviato da starfred

Ho dato un'occhiata un po' rapida e mi sembra di notare che c'è la possibilità che il main termini prima dei thread. Non vedo da nessuna parte un join().

Mmm, diciamo che il Main l'ho scritto in fretta giusto per testare le altre classi.

Originariamente inviato da starfred

Comunque è un codice molto molto molto contorto, non so se per scelta tua o per specifiche.

Scelta mia, volevo solo provare il pattern Producer-Consumer.

Originariamente inviato da starfred

Gli oggetti che devono essere usati concorrentemente non dovrebbero modificare i thread direttamente, ma dovrebbero essere i thread a modificarsi in base all'oggetto.
Tra l'altro mi sembra di capire il consumatore utilizzi la checkerHandle.cancel(true); per far terminare il loop del produttore; come prima facendo così aumenti di molto la probabilità di starvation.

Nell'idea originaria il tutto doveva attendere un singolo file, elaborarlo e poi uscire, quindi fermano il thread producer in quanto "non più necessario".

Originariamente inviato da starfred

Comunque non sto dicendo che sia sbagliato, solo che secondo me aumenti di molto la complessità e rischi di commettere parecchi errori.

Posso chiederti se hai voglia di darmi un'indicazione (a parole, pseudocodice o codice) di come avresti strutturato una cosa del genere?

Originariamente inviato da starfred

ciao

Ciao e grazie mille ad entrambi!

[banryu79]

Originariamente inviato da GByTe87

Mmm, non capisco come il producer possa rimanere in attesa del consumer, non essendoci alcuna wait() in checkFilePresence.
(Può essere che non abbia afferrato chiaramente il funzionamento di wait() e notify(). Vado a ripassarli per sicurezza. )

un thread che invochi un metodo synchronized di una istanza (la tua TestClass) deve acquisire il lock del monitor di quell'istanza prima di poter eseguire il metodo.
Una volta che il thread ha acquisito il lock del monitor dell'istanza, procede a eseguire il corpo del metodo. Quando il thread "esce" dal metodo, rilascia il lock.

Producer (il thread dello scheduled executor) e consumer (il thread che istanzia nel suo construttore) operano entrambi sulla stessa istanza di TestClass, la quale ha i metodi syncrhonized.

Ora immagina che succeda questo:

- consumer invoca printFileContent()
- consumer acquisisce il lock dell'istanza di TestClass
- consumer va in wait perchè isFilePresent vale false
- producer invoca checkFilePresence()
- producer non riesce ad aquisire il lock dell'istanza di TestClass, perchè posseduto da consumer

Risultato: consumer è in wait, l'unico thread che lo potrebbe risvegliare è producer, il quale è in attesa di poter acquisire il lock posseduto da consumer... deadlock.

[banryu79]

Originariamente inviato da starfred

Ho dato un'occhiata un po' rapida e mi sembra di notare che c'è la possibilità che il main termini prima dei thread. Non vedo da nessuna parte un join().

Questo dovrebbe essere ininfluente.
La java virtual machine non "muore" finchè ci sono thread di tipo non-demone in esecuzione.

[GByTe87]

Ok sono un pirla, nell'analizzare la tua risposta non avevo considerato il fatto che i metodi sono dichiarati synchronized.

Capito ora, grazie mille.

[starfred]

Ciao, a grandi linee io avrei fatto
Pseudo-codice

public class Producer extends Thread {
     producer(string nomefile){ }
     run(){
        TestClass.controllo(nomefile);
     }
}

public class Consumatore extends Thread {
     Consumatore (){...}
     run(){
          TestClass.leggi();
     }
}

public class TestClass {
     boolean pronto=false;
     string file_da_leggere;
     TestClass (){...}

     synchronized void controllo(string nomefile){
      file_da_leggere=nomefile; //Setto il nome del file da leggere
      pronto=true; // Memorizzo che è pronto un dato nuovo
      notify(); //Risveglio un eventuale consumatore in attesa          
      wait(); //Attendo che il consumatore abbia prelevato il dato.
     }


     synchronized void leggi(){
     
      if (!pronto) wait(); //Se non è pronto il file da leggere attendi
      /*Leggi il file_da_leggere */     
      pronto=false; //Resetto nel caso volessi riutilizzare la TestClass
      notify(); //Risveglio il produttore
     }
}

public class Main {

	public static void main(String[] args) {
		
		TestClass test = new TestClass();
		Producer producer = new Producer(test,"/home/gibbo/bla.log");
		Consumatore consumat = new Consumatore (test);
                producer.join();
                consumat.join();

	}
}

Che è la struttura classica di produttore-consumatore. In questa soluzione ho preferito che sia il produttore a fornire l'indirizzo del file da leggere. Attenzione, questo è solo nel caso io abbia un solo produttore ed un solo consumatore ed è bloccante per il produttore (cioè il produttore attende che il dato venga prelevato). Ovviamente si può fare anche una soluzione non bloccante per il produttore e si possono implementare soluzioni con più produttori/consumatori, bloccanti o non bloccanti.
Io partirei da questa struttura e poi ci costruirei sopra.

[banryu79]

@starfred: ciao, scusa, ma il tuo precedente esempio mi sembra prono a deadlock, proprio come il codice inzialmente postato da GByTe87; controllo() e leggi() sono entrambi metodi synchronized; cosa succede se un thread ottiene il lock sul monitor dell'istanza condivisa di TestClass e poi non lo rilascia perchè entra in wait?

[starfred]

E' una proprietà intrinseca nella wait(). Quando un thread esegue la wait() rilascia il monitor ed entra in un'altra coda di attesa. Quando viene eseguita la notify il thread risvegliato rientra in coda al monitor.

[banryu79]

Originariamente inviato da starfred

E' una proprietà intrinseca nella wait(). Quando un thread esegue la wait() rilascia il monitor ed entra in un'altra coda di attesa. Quando viene eseguita la notify il thread risvegliato rientra in coda al monitor.

Sì, hai ragione, mi sono completamente sbagliato.
Meglio ribadirlo a beneficio di GByTe87... chiedo scusa per l'eventuale confusione generata a causa del mio errore.

[GByTe87]

Bene, sono riuscito nel mio intento.

Producer.java

public class Producer extends Thread {
	private TestClass test;
	private String filename;

	public Producer(TestClass test, String filename) {
		this.test = test;
		this.filename = filename;
	}

	@Override
	public void run() {
		boolean end = false;

		while (end == false) {
			end = test.check(filename);
			try {
				sleep(1000);
			} catch (InterruptedException exc) {
				System.err.println(exc.getMessage());
			}
		}
	}
}

Consumer.java

public class Consumer extends Thread {
	private TestClass test;

	public Consumer(TestClass test) {
		this.test = test;
	}

	@Override
	public void run() {
		boolean end = true;

		while (end)
			end = test.read();
	}
}

TestClass.java

import java.io.File;

public class TestClass {

	String fileName;
	private boolean isFileAvailable = false;

	public synchronized boolean read() {
		System.err.println("readin");
		while (isFileAvailable == false) {
			try {
				System.err.println("[read] wait() in | isFileAvailable = " + isFileAvailable);
				wait();
				System.err.println("[read] wait() out | isFileAvailable = " + isFileAvailable);
			} catch (InterruptedException exc) {
				System.err.println(exc.getMessage());
			}
		}

		System.err.println("File pronto!");

		isFileAvailable = false;
		notify();
		System.err.println("readout");
		return isFileAvailable;
	}

	public synchronized boolean check(String filename) {

		System.err.println("checkin");

		this.fileName = filename;
		while (isFileAvailable == true) {
			try {
				System.err.println("[check] wait() in | isFileAvailable = " + isFileAvailable);
				wait();
				System.err.println("[check] wait() out | isFileAvailable = " + isFileAvailable);
			} catch (InterruptedException exc) {
				System.err.println(exc.getMessage());
			}
		}

		File file = new File(fileName);

		if (file.exists()) {
			isFileAvailable = true;
			notify();
		}
		System.err.println("checkout: isFileAvailable = " + isFileAvailable);
		return isFileAvailable;
	}
}

Comprendo che lo sleep non è il massimo dell'eleganza.
Suggermenti su qualcosa di simile da implementare per essere sicuro di aver capito?

Grazie ad entrambi

[starfred]

Ciao, la condizione while (isFileAvailable == true) della boolean check(String filename) non verrà mai eseguita per il fatto che il Producer è l'unico che può settare (isFileAvailable = true) ma quando effettua quella operazione esce dal ciclo while (end == false). Quindi, a meno che tu nel main non hai altri Producer, la condizione sopra citata è inutile.