Java - Multithreading exercise with synchronized methods

Question

I have this exercise, a kind of hospital simulation in which I have to control the accesses to each singular room. Doctors can enter the room one at the time, and can enter only if no visitors are in. A visitor, instead, can only access the room if no doctors are in it and a max of 4 more visitors are in. Here's my code:

public class Room {

public Room(){
}

public synchronized void friendVisit() throws InterruptedException{
    if(visitors>4 || doctors>0)
        wait();
    visitors++;
}

public synchronized void exitFriend(){
    visitors--;
    notify();
}

public synchronized void doctorVisit() throws InterruptedException{
    if(doctors>0 || visitors>0)
        wait();
    doctors++;
}

public synchronized void exitDoctor(){
    --doctors;
    notify();
}

public int getVisitors(){
    return visitors;
}

public int getDoctors(){
    return doctors;
}

int visitors=0; //number of visitors in the room
int doctors=0; //number of doctors in the room

Doctors and Visitors(the class it's called Friend) are threads

public class Friend extends Thread{
public Friend(Room room_reference){
    room=room_reference;
}

public void run(){
        try {
            sleep(500);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        try {
            room.friendVisit();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        try {
            sleep(500);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        room.exitFriend();
}

private Room room; //reference to the room

Here's the doctor thread:

public class Doctor extends Thread{
public Doctor(Room room_reference){
    room=room_reference;
}

public void run(){
        try {
            sleep(500);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        try {
            room.doctorVisit();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        try {
            sleep(500);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        room.exitDoctor();
}

private Room room; //reference to the room

Here's a Display thread to keep trace of the number of visitors and doctors:

public class Display extends Thread{
public Display(Room room_reference){
    room=room_reference;
}

public void run(){
    while(true)
    {
    try {
        sleep(300);
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    System.out.println("The room contains "+room.getDoctors()+
                       " doctors and "+room.getVisitors()+" visitors.");
    }
}

private Room room;

And here's my main:

public class Demo {
public static void main(String[]args){
    Room room=new Room();
    Friend friend=new Friend(room);
    Doctor doctor=new Doctor(room);
    Display display=new Display(room);
    display.start();
    while(true){
        if(new Random().nextBoolean()==true){
            friend=new Friend(room);
            friend.start();
        }
        if(new Random().nextInt(5)==3){
            doctor=new Doctor(room);
            doctor.start();
        }   
    }
}

The problem is that more than one doctor can access the room and I don't understand why, since the methods in the Room class work for the Visitors. Thanks in advance.

Answer 1

I think one of your mistakes is assuming that wait() will return only when the condition above it is satisfied:

if(doctors>0 || visitors>0)
        wait();

You may return from this call to wait() with the condition in your if statement false. Perhaps try a while loop:

while (doctors>0 || visitors>0) {
        wait();
}

(adding brackets, of course, because you know a lack of brackets is evillll.....)

There may be other problems - I've not yet fired up your code.