KoolaidLips
KoolaidLips

Reputation: 267

Threads not waking up after notifyAll() is called

The question is to create 3 threads, one prints a random number every second, if the number is even a second thread squares it and if it odd a third thread cubes it. This should occur a given number of times(in my code it is infinite, will edit that out later). My issue is that after the first iteration(i.e. a random number is created, the correct thread wakes up and does its operation) the second/third threads don't wake up after notifyAll() is called again. My code is shown below with sample output. I've added a few print statements for debugging purposes:

package com.company;
import java.util.*;


class RandomNumber implements Runnable{

int randomNum = 0;
Random rand = new Random();
boolean flag = false;

public RandomNumber() {
    Thread newThread = new Thread(this,"Random Number");
    newThread.start();
}

@Override
public synchronized void run()
{

    while(flag == false) {
        System.out.println("random num thread");
        try {
            randomNum = rand.nextInt(100) + 1;
            System.out.println(randomNum);
            flag = true;
            notifyAll();
            //System.out.println(flag);
            Thread.sleep(1000);

        } catch (Exception e) {
            System.out.println("Exception Caught");
        }

    }
}

}

class SquareNumber implements Runnable{

 RandomNumber randomNumOb;

public SquareNumber(RandomNumber randNumObject){

    this.randomNumOb = randNumObject;
    Thread squareThread = new Thread(this, "Square thread");
    squareThread.start();

}

@Override
public synchronized void run() {

    System.out.println("square thread before while");

    while(randomNumOb.flag == true) {
         System.out.println("square thread");
        if (randomNumOb.randomNum % 2 == 0)
        {
            System.out.println("Number is even so square of " + randomNumOb.randomNum + " is: " + (randomNumOb.randomNum * randomNumOb.randomNum));

            try {
                randomNumOb.flag = false;
                wait();
            }catch(Exception e){
                System.out.println("Exception caught");
            }
        }

        else {
            try {
                System.out.println("inside square else");
                wait();
            } catch (Exception e) {
                System.out.println("Exception Caught");
            }

        }
    }
    System.out.println("square thread after while");
}

}

class CubeNumber implements Runnable{

RandomNumber randomNumOb;

public CubeNumber(RandomNumber randNumObject){

    this.randomNumOb = randNumObject;
    Thread squareThread = new Thread(this, "Square thread");
    squareThread.start();

}

@Override
public synchronized void run() {
    System.out.println("cube thread before while");

    while(randomNumOb.flag == true) {
        System.out.println("cube thread");
        if (randomNumOb.randomNum % 2 == 1) {
            System.out.println("Number is odd so cube of " + randomNumOb.randomNum + " is: " + (randomNumOb.randomNum * randomNumOb.randomNum * randomNumOb.randomNum));
            try {
                randomNumOb.flag = false;
                wait();
            }catch (Exception e){

            }
        }

        else {
            try {
                System.out.println("inside cube else");
                //randomNumOb.flag = false;
                wait();
            } catch (Exception e) {
                System.out.println("Exception Caught");
            }

        }
    }
    System.out.println("cube thread after while");

}

}

public class Main {

public static void main(String[] args) {

    RandomNumber random = new RandomNumber();
    SquareNumber square = new SquareNumber(random);
    CubeNumber cube = new CubeNumber(random);
}

}

Sample output:

random num thread
81
square thread before while
square thread
inside square else
cube thread before while
cube thread
Number is odd so cube of 81 is: 531441
random num thread
68

It seems like neither the square or cube thread wakes up after this and can't figure out why. Any help would be appreciated.

Upvotes: 1

Views: 393

Answers (2)

Nathan Hughes
Nathan Hughes

Reputation: 96434

For locking and wait/notify to work there needs to be a shared lock.

There is an "intrinsic lock" which is baked into each object. The lock works as a communications hub for wait and notify. Putting synchronized on an instance method means that a thread calling that method acquires the intrinsic lock on the instance when it enters the method, and releases the intrinsic lock when it leaves. The wait/notify/notifyAll methods can be called only by a thread holding the intrinsic lock.

When a thread calls wait, that releases the lock and the thread goes dormant until it receives a notification (or gets interrupted). The lock keeps track of which threads are currently waiting on it, that's called the waitset.

When a thread calls notify, that tells the scheduler to pick a thread from the lock's waitset and send it a notification. The notifyAll method is the same except it wakes up all the other threads in the waitset.

That is how locking determines which waiting thread gets notified.

So in the posted code, each of these Runnables acquires its own intrinsic lock and there is no sharing. the wakeup notification has to be caused by another thread that has acquired the lock that the waiting thread called wait on.

Here you could create a common lock in the entrypoint class

final Object lock = new Object();  // value referenced by lock must not change

and pass it into the different Runnables in the constructor such as:

public SquareNumber(RandomNumber randNumObject, Object lock){ 
    this.lock = lock;
    ...

so they use the same lock. Then change wait and notify method calls to use that shared lock object, and change the synchronized methods to synchronized blocks that pass in the lock.

Btw about the sleep added to the RandomNumber runnable: the notifyAll doesn't take effect until the current thread releases the lock (since each waiting thread has to acquire the lock in order to leave the wait method). Sleeping here doesn't do anything to give the notification time to work, it just prevents anything from happening.

Upvotes: 5

Lutz
Lutz

Reputation: 655

CubeNumber and SquareNumber both wait all for notifications on their own object - not for notifications on the random object. So they never get notified.

package com.company;
import java.util.*;


class RandomNumber implements Runnable{

int randomNum = 0;
Random rand = new Random();
boolean flag = false;

public RandomNumber() {
    Thread newThread = new Thread(this,"Random Number");
    newThread.start();
}

@Override
public void run()
{

    while(flag == false) {
        System.out.println("random num thread");
        try {
            randomNum = rand.nextInt(100) + 1;
            System.out.println(randomNum);
            flag = true;
            synchronized(this) {
                notifyAll();
            }
            //System.out.println(flag);
            Thread.sleep(1000);

        } catch (Exception e) {
            System.out.println("Exception Caught");
        }

    }

class CubeNumber implements Runnable{

RandomNumber randomNumOb;

public CubeNumber(RandomNumber randNumObject){

    this.randomNumOb = randNumObject;
    Thread squareThread = new Thread(this, "Square thread");
    squareThread.start();

}

@Override
public synchronized void run() {
    System.out.println("cube thread before while");

    while(randomNumOb.flag == true) {
        System.out.println("cube thread");
        if (randomNumOb.randomNum % 2 == 1) {
            System.out.println("Number is odd so cube of " + randomNumOb.randomNum + " is: " + (randomNumOb.randomNum * randomNumOb.randomNum * randomNumOb.randomNum));
            try {
                randomNumOb.flag = false;
                synchronised(randomNumOb) {
                    randomNumOb.wait();
                }
            }catch (Exception e){

            }
        }

        else {
            try {
                System.out.println("inside cube else");
                //randomNumOb.flag = false;
                wait();
            } catch (Exception e) {
                System.out.println("Exception Caught");
            }

        }
    }
    System.out.println("cube thread after while");

}

same with the square version.

But the added problem is that you have no guaranty that both the cube and the square methods will run within the sleep second of randomnumber. Possibly a sempahor might be of use here.

Upvotes: 0

Related Questions