Yuval Levy
Yuval Levy

Reputation: 2516

can I use static boolean variable as a lock for a synchronized thread?

I tried to use static boolean variable to lock and unlock two synchronized threads. So I wrote the following code:

public class Main {

    public static void main(String[] args){

        //MyObject lock = new MyObject();
        Thread1 t1 = new Thread1(100,'#');
        Thread1 t2 = new Thread1(100,'*');

        t1.start();
        t2.start();
    }
}

public class Thread1 extends Thread {

    public static boolean lock;
    int myNum;
    char myChar;

    public Thread1(int num, char c){
        myNum = num;
        myChar = c;
        lock = false;
    }

    public synchronized void run(){
        System.out.println(getName() + " is runing");
        while (Thread1.lock == true){
            System.out.println(getName() + " is waiting");
            try{wait();}
            catch(InterruptedException e){}
        }
        Thread1.lock = true;
        for(int i = 0; i<myNum; i++){
            if(i%10==0)
                System.out.println("");
            System.out.print(myChar);
        }
        Thread1.lock = false;
        notifyAll();
    }
}

probably I am not doing it right, cause only one thread is printing the "mychar" and the other thread is just going into wait() and not waking up when I do the notifyAll(). I thought that this can be a good way to use a static boolean variable for the whole class and than change it every time and call notifyAll() to check this flag in the others objects...

output example:

Thread-0 is runing

Thread-1 is runing
Thread-1 is waiting
##########
##########
##########
##########
##########
##########
##########
##########
##########
##########

Upvotes: 4

Views: 2685

Answers (3)

Jamie Cockburn
Jamie Cockburn

Reputation: 7555

Why it doesn't work

notify() and wait() work with the "monitor" of the object they are called on. In your case, that is the this, the particular instance of Thread1 that is running.

So, when Thread-0 runs:

  • it checks lock
  • finds it false
  • it runs the main code
  • it calls notifyAll() on this (which is itself, Thread-0).

Thread-1 runs:

  • it checks lock
  • finds that it is true
  • and calls wait() on this (which is itself, Thread-1)

Since Thread-0 calls notifyAll() on this (which is itself), and Thread-1 calls wait() on this (which is itself), Thread-1 is waiting on a different monitor than Thread-0 is notifying, so it is never released.

Solution

If your plan is to just get that code to run sequentially, use this:

public class Thread1 extends Thread {

    private static final Object lock = new Object();

    public void run(){
        // non-sequential code
        System.out.println(getName() + " is running");
        synchronized (lock) {
            // this code will be run sequentially by one thread at a time
        }
        // non-sequential code
    }
}

Upvotes: 4

Alfredo Diaz
Alfredo Diaz

Reputation: 658

You can use a simpler way to synchronize the threads.

class Thread1 extends Thread {

    private static final Object lock = new Object();
    int myNum;
    char myChar;

    public Thread1(int num, char c){
        myNum = num;
        myChar = c;
    }

    public void run(){
        System.out.println(getName() + " is runing");
        synchronized(lock) {
            for(int i = 0; i<myNum; i++){
                if(i%10==0)
                    System.out.println("");
                System.out.print(myChar);
            }
        }
    }
}

Upvotes: 0

Ordous
Ordous

Reputation: 3884

Sorry, but this provides negative value, because it has no guarantees and only gives an illusion of safety. There are several issues with this:

  1. The variable is non-volatile, as such changes from one thread may not be visible to the other.
  2. Your locking is not atomic. Hence what could happen is both threads can pass the while loop, and then both set the boolean to true.
  3. notifyAll() wakes up all threads waiting on the object instance. And since each thread is waiting on itself as the monitor, they are not getting woken up.

There are plenty of good locking mechanisms in Java - try the ones in the concurrency package (Though in your case a simple synchronized block is enough). If you insist on using a boolean for a lock, you would need to:

  1. Make a static final AtomicBoolean as you signaler (non final is fine as long as you don't change it and guarantee visibility)
  2. Use compareAndSet as your looping/locking condition so that access is atomic.
  3. Wait() on the AtomicBoolean so that the threads share the monitor they are waiting on (and notifyAll() on it as well).

Upvotes: 2

Related Questions