wait() and notify() + synchronized method

[pEkvo]

I'm starting out with some easy exercises regarding threads, synchronization, etc. But I'm stuck with this one. I have the following two classes:

public class Carwash {
    private static boolean busy;

    public synchronized void carArrival(int carNumber) {
        if (busy) {
            System.out.println("Car nr " + carNumber + " must wait!");
            try {
                wait();
            } catch (InterruptedException e) {
                // niks
            }
        }
        busy = true;
        System.out.println("Start cleaning car nr " + carNumber);
        try {
            Thread.sleep(1600);
        } catch (InterruptedException e) {
            // niks
        }
        carDeparture(carNumber);
    }

    public synchronized void carDeparture(int wagenNr) {
        System.out.println("Car nr " + wagenNr + " is done");
        busy = false;
        notifyAll();
    }
}

public class Car implements Runnable {
    private int carNumber;
    private int arrivalTime;
    private Carwash carwash;

    public Car(int wagenNr, int arrivalTime, Carwash carwash) {
        this.carNumber = wagenNr;
        this.arrivalTime = arrivalTime;
        this.carwash = carwash;
    }

    public void run() {
        try {
            Thread.sleep(1000 * arrivalTime);
        } catch (InterruptedException e) {
            // niks
        }

        System.out.println("Car nr " + carNumber + " has arrived.");
        carwash.carArrival(carNumber);
    }
}

And my main method:

public class RunCarwash {
    public static void main(String[] args) {
        Carwash carwash = new Carwash();

        Car[] cars = {  new Car(1, 0, carwash),
                        new Car(2, 1, carwash),
                        new Car(3, 3, carwash),
                        new Car(4, 7, carwash),
                        new Car(5, 8, carwash),
                        new Car(6, 9, carwash) };

        for (Car car : cars)
            new Thread(car).start();
    }

}

The code outputs the following:

Car nr 1 has arrived.
Start cleaning car nr 1
Car nr 2 has arrived.
Car nr 1 is done
Start cleaning car nr 2
Car nr 3 has arrived.
Car nr 2 is done
Start cleaning car nr 3
Car nr 3 is done
Car nr 4 has arrived.
Start cleaning car nr 4
Car nr 5 has arrived.
Car nr 4 is done
Start cleaning car nr 5
Car nr 6 has arrived.
Car nr 5 is done
Start cleaning car nr 6
Car nr 6 is done

The output is correct in the sense that a new car can't start until the previous car has finished. But I can't seem to print out the message in the Carwash class saying that a particular car has to wait. So for instance, after car 2 has arrived, it should display the message that car 2 has to wait because car 1 has not yet finished.

The reason I believe this is happening is because the method is synchronized which means the other Car thread doesn't get access to the method until the car is actually done, so it never encounters the wait() statement, it just waits because the method is already locked. I have no idea how to fix this though...

[Norm]

Try adding some more printlns to see when the methods are called and what the values of all the variables are at that time.

[Norm]

Can you explain what you want the program to do that is different from what it is doing now?

Where do you want the waiting threads to queue up while they wait?

[pEkvo]

I want the queueing cars (threads) to wait inside the carArrival method until the car is ready to be cleaned. And according to me they are only ready to be cleaned when the previous one has departed, by calling the notifyAll() method in the carDeparture method, I thought I would allow the next car to execute the carArrival method from the wait() point.

[Norm]

If you want the Cars to queue at the wait() call, then you will have to let them into the method.
One way would be to remove the synchronized modifier and make a local synchronized code block .

[pEkvo]

I tried what you suggested:

public void carArrival(int carNumber) {
        if (busy) {
            System.out.println("Car nr " + carNumber + " must wait!");
            try {
                wait();
            } catch (InterruptedException e) {
                // niks
            }
        }
        synchronized (this) {
            busy = true;
            System.out.println("Start cleaning car nr " + carNumber);
            try {
                Thread.sleep(1600);
            } catch (InterruptedException e) {
                // niks
            }
            carDeparture(carNumber);
        }

    }

And it then does print out the "car # must wait!" line, but it also throws an IllegalStateMonitorException if car arrives before the previous one was done.

[Norm]

IllegalStateMonitorException

Did you read the API doc for why that exception is thrown?

[pEkvo]

Yes, but I don't think I understand it properly. According to me it means that when one of the Car threads calls the wait() method, it doesn't know what it is waiting for (and I think this is because the wait() method is not in a synchronized block, but that just brings me back to the original solution).

[Norm]

I don't think I understand it properly.

Copy it here with your questions.

Look at the API doc for the wait() method. It lists the exceptions it throws and the reason it throws them.

because the wait() method is not in a synchronized block,

Yes, you need to put the wait in a synchronized block. See the API doc for the wait method.

[pEkvo]

Ok, I solved it. The problem was that I put the Thread.sleep(1600) statement INSIDE the carArrival method. This sleep is meant to simulate the cleaning of the car. By moving that code block to the thread run method, the thread leaves the carArrival method and allows other threads to start queueing in the carArrival method.

Thanks for your help. :-)