Thread: Java Multithreading Example - Issues

Hi,

Problem statement - 5 visitors come to a car showroom having 3 cars. The visitors take a test drive for random time and the other visitors have to wait till a car becomes free. This has to be implemented using Threads.

I have written the code and it works fine. But, sometimes the output is wierd.

Code -

import java.util.Random;

class VisitorSchool implements Runnable {
DrivingSchool drivingSchool;

public VisitorSchool(DrivingSchool drivingSchool) {
this.drivingSchool = drivingSchool;
}

public void run() {
if (CarsGreaterThanZero()) {
getInTheCarAndDrive();
} else if (CarsEqualToZero()) {
waitForCar();
getInTheCarAndDrive();
}
}

public boolean CarsGreaterThanZero() {
synchronized (drivingSchool) {
if (drivingSchool.getNoOfCars() > 0) {
return true;
} else {
return false;
}
}
}

public boolean CarsEqualToZero() {
synchronized (drivingSchool) {
if (drivingSchool.getNoOfCars() == 0) {
return true;
} else {
return false;
}
}
}

void waitForCar() {
synchronized (drivingSchool) {
try {
drivingSchool.wait();
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}

void getInTheCarAndDrive() {
synchronized (drivingSchool) {
drivingSchool.setNoOfCars(drivingSchool.getNoOfCar s() - 1);
System.out.println(Thread.currentThread().getName( )
+ " is driving, " + drivingSchool.getNoOfCars()
+ " available");
}
Random numGen = new Random();
int r = numGen.nextInt(1000);
try {
Thread.sleep(r);
} catch (InterruptedException e) {
e.printStackTrace();
}
synchronized (drivingSchool) {
drivingSchool.setNoOfCars(drivingSchool.getNoOfCar s() + 1);
System.out.println(Thread.currentThread().getName( )
+ " has done driving, " + drivingSchool.getNoOfCars()
+ " available");
drivingSchool.notify();
}
}
}

public class DrivingSchool {
private int noOfCars = 3;

public int getNoOfCars() {
return noOfCars;
}

public void setNoOfCars(int noOfCars) {
this.noOfCars = noOfCars;
}

public static void main(String[] args) {
DrivingSchool drivingSchool = new DrivingSchool();
VisitorSchool vs1 = new VisitorSchool(drivingSchool);
Thread t1 = new Thread(vs1, "A");
VisitorSchool vs2 = new VisitorSchool(drivingSchool);
Thread t2 = new Thread(vs2, "B");
VisitorSchool vs3 = new VisitorSchool(drivingSchool);
Thread t3 = new Thread(vs3, "C");
VisitorSchool vs4 = new VisitorSchool(drivingSchool);
Thread t4 = new Thread(vs4, "D");
VisitorSchool vs5 = new VisitorSchool(drivingSchool);
Thread t5 = new Thread(vs5, "E");
t1.start();
t2.start();
t3.start();
t4.start();
t5.start();
}
}



Acceptable Output -

A is driving, 2 available
C is driving, 1 available
E is driving, 0 available
A has done driving, 1 available
B is driving, 0 available
B has done driving, 1 available
D is driving, 0 available
E has done driving, 1 available
C has done driving, 2 available
D has done driving, 3 available



Unacceptable Output -

A is driving, 2 available
C is driving, 1 available
B is driving, 0 available
D is driving, -1 available
D has done driving, 0 available
C has done driving, 1 available
B has done driving, 2 available
A has done driving, 3 available

Please let me know what is the bug in my code and suggest the solution.

Thanks and Regards,
Vijay

 
import java.util.Random;
 
class VisitorSchool implements Runnable {
	DrivingSchool drivingSchool;
 
	public VisitorSchool(DrivingSchool drivingSchool) {
		this.drivingSchool = drivingSchool;
	}
 
	public void run() {
		if (CarsGreaterThanZero()) {
			getInTheCarAndDrive();
		} else if (CarsEqualToZero()) {
			waitForCar();
			getInTheCarAndDrive();
		}
	}
 
	public boolean CarsGreaterThanZero() {
		synchronized (drivingSchool) {
			if (drivingSchool.getNoOfCars() > 0) {
				return true;
			} else {
				return false;
			}
		}
	}
 
	public boolean CarsEqualToZero() {
		synchronized (drivingSchool) {
			if (drivingSchool.getNoOfCars() == 0) {
				return true;
			} else {
				return false;
			}
		}
	}
 
	void waitForCar() {
		synchronized (drivingSchool) {
			try {
				drivingSchool.wait();
			} catch (InterruptedException e) {
				e.printStackTrace();
			}
		}
	}
 
	void getInTheCarAndDrive() {
		synchronized (drivingSchool) {
			drivingSchool.setNoOfCars(drivingSchool.getNoOfCars() - 1);
			System.out.println(Thread.currentThread().getName()
					+ " is driving, " + drivingSchool.getNoOfCars()
					+ " available");
		}
		Random numGen = new Random();
		int r = numGen.nextInt(1000);
		try {
			Thread.sleep(r);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
		synchronized (drivingSchool) {
			drivingSchool.setNoOfCars(drivingSchool.getNoOfCars() + 1);
			System.out.println(Thread.currentThread().getName()
					+ " has done driving, " + drivingSchool.getNoOfCars()
					+ " available");
			drivingSchool.notify();
		}
	}
}
 
public class DrivingSchool {
	private int noOfCars = 3;
 
	public int getNoOfCars() {
		return noOfCars;
	}
 
	public void setNoOfCars(int noOfCars) {
		this.noOfCars = noOfCars;
	}
 
	public static void main(String[] args) {
		DrivingSchool drivingSchool = new DrivingSchool();
		VisitorSchool vs1 = new VisitorSchool(drivingSchool);
		Thread t1 = new Thread(vs1, "A");
		VisitorSchool vs2 = new VisitorSchool(drivingSchool);
		Thread t2 = new Thread(vs2, "B");
		VisitorSchool vs3 = new VisitorSchool(drivingSchool);
		Thread t3 = new Thread(vs3, "C");
		VisitorSchool vs4 = new VisitorSchool(drivingSchool);
		Thread t4 = new Thread(vs4, "D");
		VisitorSchool vs5 = new VisitorSchool(drivingSchool);
		Thread t5 = new Thread(vs5, "E");
		t1.start();
		t2.start();
		t3.start();
		t4.start();
		t5.start();
	}
}

this is working fine.

package com.oracle;
 
import java.util.Random;
 
class Visitor implements Runnable {
	ThreadExample t;
 
	Visitor(ThreadExample t) {
		this.t = t;
	}
 
	public void run() {
		if (t.getNoOfCars() == 0) {
			synchronized (t) {
				try {
					t.wait();
				} catch (InterruptedException e) {
					e.printStackTrace();
				}
			}
		}
		synchronized (t) {
			t.setNoOfCars(t.getNoOfCars() - 1);
			System.out.println(Thread.currentThread().getName()
					+ " is driving, " + t.getNoOfCars() + " car(s) available");
		}
		Random numGen = new Random();
		int r = numGen.nextInt(5000);
		try {
			Thread.sleep(r);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
		synchronized (t) {
			t.setNoOfCars(t.getNoOfCars() + 1);
			System.out.println(Thread.currentThread().getName()
					+ " has done driving, " + t.getNoOfCars() + " car(s) available");
			t.notify();
		}
	}
}
 
public class ThreadExample {
	private int noOfCars = 3;
 
	public int getNoOfCars() {
		return noOfCars;
	}
 
	public void setNoOfCars(int noOfCars) {
		this.noOfCars = noOfCars;
	}
 
	public static void main(String[] args) {
		ThreadExample t = new ThreadExample();
		System.out.println(t.getNoOfCars() + " cars are available");
		Visitor v1 = new Visitor(t);
		Visitor v2 = new Visitor(t);
		Visitor v3 = new Visitor(t);
		Visitor v4 = new Visitor(t);
		Visitor v5 = new Visitor(t);
		Thread t1 = new Thread(v1, "A");
		Thread t2 = new Thread(v2, "B");
		Thread t3 = new Thread(v3, "C");
		Thread t4 = new Thread(v4, "D");
		Thread t5 = new Thread(v5, "E");
		t1.start();
		t2.start();
		t3.start();
		t4.start();
		t5.start();
	}
}

Problem recreated as stated from your original post

suggested Unacceptable Output thread route
Thread A to .sleep and Thread B to just about to call getInTheCarAndDrive() thread switch and run
Thread C to .sleep and Thread D to just about to call getInTheCarAndDrive() thread switch back to
Thread B to .sleep and Thread D to .sleep
Thread E to .wait

So the issue that you seemed to have was one of an:
Thread D: oops I didn't keep the "check ok to run" and "perform task" in the same sync block. Thus allowing thread switches between the check and the task to allow the other threads to pass through and take the resource that I thought was ok to use... I'll use a resource anyway.

A quick fix of your old code:

void getInTheCarAndDrive() {
		synchronized (drivingSchool) {
			while(!CarsGreaterThanZero()) waitForCar();//add this line
			drivingSchool.setNoOfCars(drivingSchool.getNoOfCars() - 1);