How to correctly use signaling via shared objects?

Question

I'm currently studying about signaling in threads and came across this article for signaling via shared objects,
http://tutorials.jenkov.com/java-concurrency/thread-signaling.html

It says that we can create a shared object and pass that object to threads which, threads can use to signal each other.

Following is the snippet provided for shared object,

public class MySignal{
  protected boolean hasDataToProcess = false;

  public synchronized boolean hasDataToProcess(){
    return this.hasDataToProcess;
  }

  public synchronized void setHasDataToProcess(boolean hasData){
    this.hasDataToProcess = hasData;  
  }
}

I tried to use it in my class as,

class MySignal {
    boolean hasDataToProcess = false;

    public MySignal(boolean defaultValue) {
        this.hasDataToProcess = defaultValue;
    }

    public synchronized boolean hasDataToProcess() {
        return this.hasDataToProcess;
    }

    public synchronized void setHasDataToProcess(boolean hasDataToProcess) {
        this.hasDataToProcess = hasDataToProcess;
    }
}

class MyThreadRunnable implements Runnable {
    MySignal sharedSignal;

    MyThreadRunnable(MySignal signal) {
        this.sharedSignal = signal;
    }

    @Override
    public void run() {
        System.out.println(Thread.currentThread().getName() + " starts running..");

        synchronized (sharedSignal) {
            System.out.println(Thread.currentThread().getName() + " accessing sharedSignal");

            while(sharedSignal.hasDataToProcess()) {
                sharedSignal.setHasDataToProcess(false);
                try {
                      System.out.println(Thread.currentThread().getName() + " going to sleep");
                      Thread.sleep(3000);
                } catch (InterruptedException e) {
                }
            }

            sharedSignal.setHasDataToProcess(true);
            System.out.println(Thread.currentThread().getName() + " ended.");
        }
    }

}

public class Test2 {
    public static void main(String[] args) {
        MySignal mySignal = new MySignal(true);

        MyThreadRunnable t1 = new MyThreadRunnable(mySignal);
        MyThreadRunnable t2 = new MyThreadRunnable(mySignal);

        Thread t3 = new Thread(t1);
        Thread t4 = new Thread(t2);

        t3.start();
        t4.start();
    }
}

This provided the expected output as,

Thread-1 starts running..
Thread-0 starts running..
Thread-1 accessing sharedSignal
Thread-1 going to sleep
Thread-1 ended.
Thread-0 accessing sharedSignal
Thread-0 going to sleep
Thread-0 ended.

But even if I remove the synchronized on the MySignal methods, this provides the same output as sharedSignal object is locked by one of the threads. And, if I remove only the synchronized in run(), it does not work properly as one of the threads end before even going to sleep.

So this code is only running correctly due to the lock on sharedSignal object.

Is this how the signaling has to be used?
My intuition says that I've missed something. I tried searching for a good example but no luck so far. Any help would be appreciated.

Answer 1

But even if I remove the synchronized on the MySignal methods, this provides the same output as sharedSignal object is locked by one of the threads

Removing the synchronized from the methods won't make a difference as there is already a synchronized block guarding the method access from different threads.

And, if I remove only the synchronized in run(), it does not work properly as one of the threads end before even going to sleep.

But if you remove the the synchronized block then the contents of the block are not executed in an atomic way.

What I mean is without the synchronized block the any thread can call the sharedSignal.hasDataToProcess() get the lock on the MySignal object and then release it after it is done with the method then another thread is free to call the sharedSignal.setHasDataToProcess(false); as the lock on the MySignal instance was already released by the earlier thread when it was done with the method.

//Acquires lock for the entire block
synchronized (sharedSignal) { 
     System.out.println(Thread.currentThread().getName() + " accessing sharedSignal");
     while(sharedSignal.hasDataToProcess()) {
           sharedSignal.setHasDataToProcess(false);
             try {
                System.out.println(Thread.currentThread().getName() + " going to sleep");
                Thread.sleep(3000);
             } catch (InterruptedException e) {
             }
      }        
      sharedSignal.setHasDataToProcess(true);
      System.out.println(Thread.currentThread().getName() + " ended.");
}

Now without the synchronized block, the code of the block is not executed in an atomic way:

System.out.println(Thread.currentThread().getName() + " accessing sharedSignal");
//say thread1 acquires lock here
while(sharedSignal.hasDataToProcess()) {
     //thread1 releases lock here, thread2 can acquire lock on the same object
     sharedSignal.setHasDataToProcess(false);
        try {
            System.out.println(Thread.currentThread().getName() + " going to sleep");
            Thread.sleep(3000);
         } catch (InterruptedException e) {
         }
   }        
   sharedSignal.setHasDataToProcess(true);
   System.out.println(Thread.currentThread().getName() + " ended.");
}