I'm working on an excercise about threads in java and I'm required to do the following: Suppose you have two threads, let's say player1 and player2, both sharing a Ball object. The goal is to make one of them have the ball for 1 second, then drop it and make the second player do the same thing, a couple of times. More specifically, I have two classes like this (Player2 is the same, except for the name):
class Player1 extends Thread{
private Ball ball;
public Player1 (Ball ball) {
this.ball=ball;
this.setName("Player 1");
}
@Override
public void run() {
for (int i=0;i<25;i++){
try{
ball.takeBall(); //Player takes the ball
sleep(1000);
ball.dropBall(); //Player drops the ball
}catch (Exception e){
}
}
}
}
The goal is to write the methods takeBall() and dropBall()in a way that makes the two threads alternate.
This is my attempt.
public class Ball{
boolean ballInUse = false;
public synchronized void takeBall() throws InterruptedException {
System.out.println("Ball taken by -> "+Thread.currentThread().getName());
while (ballInUse){
wait();
}
ballInUse = true;
}
public synchronized void dropBall() throws InterruptedException {
System.out.println("Ball dropped by -> "+Thread.currentThread().getName());
ballInUse = false;
notify();
}
}
//Edited to show the main class
class MyMainClass
public static void main(String args[]) {
Ball ball = new Ball();
Player1 player1 = new Player1(ball);
Player2 player2 = new Player2(ball);
player1.start();
player2.start();
}// main
}
It kind of makes sense in my head that since the variable ballInUse only becomes false after the ball has been dropped by the other thread, the two threads should alternate between these two methods.
Obviously there's something wrong with that idea, since this is what I'm getting:
Ball taken by -> Player 1
Ball taken by -> Player 2
Ball dropped by -> Player 1
Ball taken by -> Player 1
Ball dropped by -> Player 1
Ball taken by -> Player 1
Ball dropped by -> Player 1
Ball taken by -> Player 1
It seems like the second thread never gets notified from the second method. I'm surely missing an important point here, so any hints would be appreciated.
Sure it is. However, 'wake the thread' does not mean 'it will fly ahead and IMMEDIATELY start running this very instant in time, whilst your thread has to do the extremely slow move of.. looping a for loop'.
It could (that's one of the problems of threads: Things are arbitrary, and arbitrary is bad, because it's untestable. There is no fix for this, other than not using threads that require synchronization, for example by doing all state communication via transactions and a database)... but it usually won't.
Your dropBall runs to its natural end, then it releases the lock (as the dropBall method is synchronized). Only now can the other thread's takeBall move on from its wait (wait doesn't JUST wait for a notify - it also releases the lock and can't continue until it has both been notified AND it can re-acquire the lock, which initially it by definition cannot, because you can't notify without holding the lock).
What actually happens is this:
Locks are not guaranteed to be 'fair'. Because fair locks take time and the JVM is designed to fulfill all guarantees as fast as possible (and thus, doesn't give you unpromised freebies), JVM locks as a consequence are pretty much never fair except on extremely bizarro combinations of JVM impl, OS, and hardware.
Shove a sleep in between the dropping and the re-taking and you'll likely see something you were expecting. Now the VM/OS/hardware either [A] actually lets player2's thread grab the lock and actually take the ball, or [B] twiddle its thumbs.
Note that the Thread.sleep you'll be writing MUST NOT be in any place where you hold the lock. Given that the entirety of the takeBall method is synchronized, don't do it there; do it in your for loop. (And as a side note, catch an exception and do nothing? That is extremely bad, don't ever do that1 - I fixed that for you too):
try {
ball.takeBall();
sleep(1000);
ball.dropBall();
sleep(100);
} catch (Exception e) {
throw new RuntimeException("Unhandled", e);
}
[1] Unless you truly intend for code to silently ignore the error and carry on like nothing happened. There are times where that is the right move but extremely rare and not something a newbie should be meddling with. Far too easy to misunderstand something and then get utterly lost because crucial information (namely, that an error occured, where it occurred, why, and the details surrounding) got discarded.