Java - Problem with Threads

Question

I have a class Producer and a class Printer. The producers read from a file data to create PrinttJobs objects. A Producer generates one PrintJob and add to a Queue, notifying the Printer. Producer than waits 1 - 5 seconds to create new PrintJobs. When the Printer is notyfied it turns on and get the jobs from the queue and print them. In this period Producer can't work. Printer prints everything and wait again, letting the Producer work again. The app works with 2 Producers and 1 Printer. My problem is that sometimes it go well, sometimes it doens't print produce everything. Also I think that my wait with the time limit 1-5 seconds is not working well/ may be the problem. Code is below:

EDITED

When the Producers actually produce something, they send at the same time almost always. And sometimes it stop producing but still data in the file.

class Printer implements Runnable {

    protected long MILLIS_PER_PAGE = 500;

    private String name;
    Queue queue;
    boolean lock = false;

    public Printer(String name, Queue queue) {
        this.name = name;
        this.queue = queue;
    }

    public String getPrinterName() {
        return name;
    }

    @Override
    public void run() {
        System.out.println("["+getPrinterName()+"] Ligando...");
        while(true) {
            synchronized(this){
                if(queue.isEmpty()) {
                    try {
                        System.out.println("["+getPrinterName()+"] Esperando por tabalho de impressão...");
                        lock = false;
                        halt();
                    } 
                    catch (InterruptedException e) {
                        e.printStackTrace();
                    }

                }
                else {
                    lock = true;
                    PrintJob pj = queue.removeFront();
                    System.out.println("Imprimindo "+ pj.getJobName());
                    try {
                        wait(pj.getNumberOfPages() * MILLIS_PER_PAGE);
                        System.out.println(pj.getJobName() + " ok.");
                    } 
                    catch (InterruptedException e) {
                        e.printStackTrace();
                    }


                }
            }
        }
    }

    public void halt() throws InterruptedException {

        wait();

    }

}
`

`
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.util.Scanner;

class Producer implements Runnable {

    private String name;
    Queue queue;

    String job;
    int pags;
    String arquivo;

    public Producer(String name, Queue queue, String arquivo) {
        this.name = name;
        this.queue = queue;
        this.arquivo = arquivo;
    }

    public String getProducerName() {
        return name;
    }

    @Override
    public void run(){
        synchronized (PrinterApp.printer) {

            FileReader fin;

            try {
                fin = new FileReader(arquivo);
                Scanner src = new Scanner(fin);

                while (src.hasNext() ) {
                    if (PrinterApp.printer.lock == true){
                        PrinterApp.printer.wait();
                    }
                    job = src.next();
                    pags = src.nextInt();
                    PrintJob p = new PrintJob(job, pags);

                    queue.addBack(p);

                    System.out.println("["+getProducerName()+"] produzindo arquivo " + job +", número de páginas: " + pags);
                    PrinterApp.printer.notify();
                    PrinterApp.printer.wait(1000 + (int)Math.round((Math.random() * (5000 - 1000))));  

                }
                fin.close();
            }


            catch (FileNotFoundException e) {
                e.printStackTrace();

            } 
            catch (QueueException e) {

                e.printStackTrace();
            }

            catch (IOException e) {
                e.printStackTrace();
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

        }
    }
}

Answer 1

The problem is with the following

                if (PrinterApp.printer.lock == true){
                    PrinterApp.printer.wait();
                }

lock may not be true after wait ends. Waits should always be in a loop.

Also the printer never notifies the producers that lock has changed. You should call notify before calling wait in the printer.

If this isn't for isn't homework, then I'd recommend using a blocking queue which will handle all the waits and notifies for you.

class Printer implements Runnable {

protected long MILLIS_PER_PAGE = 500;

private String name;
Queue queue;
boolean lock = false;

public Printer(String name, Queue queue) {
    this.name = name;
    this.queue = queue;
}

public String getPrinterName() {
    return name;
}

@Override
public void run() {
    System.out.println("["+getPrinterName()+"] Ligando...");
    while(true) {
        synchronized(this){
            if(queue.isEmpty()) {
                try {
                    System.out.println("["+getPrinterName()+"] Esperando por tabalho de impressão...");
                    lock = false;
                    notifyAll();
                    halt();
                } 
                catch (InterruptedException e) {
                    e.printStackTrace();
                }

            }
            else {
                lock = true;
                PrintJob pj = queue.removeFront();
                System.out.println("Imprimindo "+ pj.getJobName());
                try {
                    wait(pj.getNumberOfPages() * MILLIS_PER_PAGE);
                    System.out.println(pj.getJobName() + " ok.");
                } 
                catch (InterruptedException e) {
                    e.printStackTrace();
                }


            }
        }
    }
}

public void halt() throws InterruptedException {

    wait();

}

}
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.util.Scanner;

class Producer implements Runnable {

private String name;
Queue queue;

String job;
int pags;
String arquivo;

public Producer(String name, Queue queue, String arquivo) {
    this.name = name;
    this.queue = queue;
    this.arquivo = arquivo;
}

public String getProducerName() {
    return name;
}

@Override
public void run(){
    FileReader fin;

    try {
         fin = new FileReader(arquivo);
         Scanner src = new Scanner(fin);

         while (src.hasNext() ) {                    
         synchronized (PrinterApp.printer) {
                while (PrinterApp.printer.lock == true){
                    PrinterApp.printer.wait();
                }
                job = src.next();
                pags = src.nextInt();
                PrintJob p = new PrintJob(job, pags);

                queue.addBack(p);

                System.out.println("["+getProducerName()+"] produzindo arquivo " + job +", número de páginas: " + pags);
                PrinterApp.printer.notifyAll();
             }
             // don't wait here since your not waiting on a condition to change
             Thread.sleep(1000 + (int)Math.round((Math.random() * (5000 - 1000))));  

            }
            fin.close();
        }


        catch (FileNotFoundException e) {
            e.printStackTrace();

        } 
        catch (QueueException e) {

            e.printStackTrace();
        }

        catch (IOException e) {
            e.printStackTrace();
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

    }
}
}