Facing issue in trying to synchronize two threads

Question

I was trying to synchronize to two threads, one printing "ping" and another printing "pong", to print something like "ping pong ping pong ping...." I wrote the following program to achieve this, but couldn't get the expected result. I wonder what mistake I'm making. Solutions for the same are available on the internet but I thought I should give it a try myself before looking for a readily available answer.

class MyThread implements Runnable{
  String val = null;
  MyThread(String val) {
      this.val = val;
  }

    public void run() {
        while(true){
            PingPong.pintMessage(val);
        }
    }
}

public class PingPong {
   static Integer turn = 1;

   public static void main(String args []) {
       Thread t1 = new Thread(new MyThread("ping"));
       Thread t2 = new Thread(new MyThread("pong"));
       t1.start();
       t2.start();

   }
   public static void pintMessage(String msg) {
       synchronized (turn) {
         if(turn==1) {
             System.out.println(Thread.currentThread().getName()+" "+msg);
             turn=2;
         }
         else {
             System.out.println(Thread.currentThread().getName()+" "+msg);
             turn = 1;
         }
    }
   }
}

Output :

Thread-0 ping
Thread-1 pong
Thread-1 pong
Thread-1 pong
Thread-0 ping
Thread-0 ping
Thread-0 ping
Thread-1 pong
Thread-1 pong
Thread-1 pong
Thread-0 ping
Thread-0 ping
Thread-0 ping
Thread-1 pong
Thread-1 pong
Thread-1 pong
Thread-0 ping
Thread-0 ping
Thread-0 ping
Thread-1 pong
Thread-1 pong
Thread-1 pong
Thread-0 ping
Thread-0 ping
Thread-0 ping
Thread-1 pong
Thread-1 pong
Thread-1 pong
Thread-0 ping
Thread-0 ping
Thread-0 ping
Thread-1 pong

Kindly advise what fundamental mistake I'm making here and if there is a better approach to achieve the same behaviour. Thanks !!

After making suggested changes : I have tried to use thread communication using wait/notify but getting IllegalMonitorStateException. I guess I have used wait/notify on the object I am acquiring lock on(wait/notify used in synchronized context). Please help me understand the subtlety of the concept. Thnaks !

class MyThread implements Runnable{
    String val = null;

    MyThread(String val) {
        this.val = val;
    }

    public void run() {
        while(true){
            try {
                PingPong.printMessage(val);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }

        }
    }
}

public class PingPong {
    static Integer turn = 1;

    public static void main(String args []) {
        Thread t1 = new Thread(new MyThread("ping"));
        Thread t2 = new Thread(new MyThread("pong"));
        t1.start();
        t2.start();
    }
    public static void printMessage(String msg) throws InterruptedException {
        synchronized (turn) {
            if(turn==1) {
                System.out.println(Thread.currentThread().getName()+" "+msg);
                turn=2;
            }
            else {
                System.out.println(Thread.currentThread().getName()+" "+msg);
                turn = 1;
            }
            turn.notifyAll();
            turn.wait();
        }
    }
}

Output :
Thread-0 ping
Exception in thread "Thread-0" Thread-1 pong
java.lang.IllegalMonitorStateException
    at java.lang.Object.notifyAll(Native Method)
    at PingPong.printMessage(PingPong.java:40)
    at MyThread.run(PingPong.java:12)
    at java.lang.Thread.run(Unknown Source)

Hi Scary, I just edited the question to reflect the obtained output. Thanks ! — user2532344, Oct 24 '16 at 07:06
`printMessage` doesn't guarantee that your ping thread will always have a turn number of 1 and pong has a turn number of 2. As far as the method is concerned the turn number is unrelated to the current thread executing. To get the desired output, you need to find a way to communicate between threads so that ping knows its turn — kjsebastian, Oct 24 '16 at 07:07
@conscells : It would be helpful if you could suggest a way to allow ping thread to know if it's turn only. — user2532344, Oct 24 '16 at 07:16
I didn't mention because you said you did not want a readily available answer. But you could look into thread signaling or blocking queues to make multiple threads cooperate. I would also recommend going through http://tutorials.jenkov.com/java-concurrency/index.html. — kjsebastian, Oct 24 '16 at 07:22

Timothy Truckle · Answer 1 · 2016-10-24T07:30:30.043

you got the concept of synchonized wrong.

The JVM ensures, that there is allways only on process executing the synchronised block, but it does not control the enqueing process, that means it only handles the calls in the order they try to call that block.

In your case: each thread has the chance to execute the loop multiple times before the JVM sets it to background for another thread to execute.

If you want to ensure a particular order you have to use wait() and notify()/notifyAll(): ([edit]: this code might not working correctly (not tested), it is just to demonstrate the principle...)

class MyThread implements Runnable{
  private Object other;
  public void setOther (MyThread other){
    this.other = other;

  }
  //no changes
       while(true){
            PingPong.pintMessage(val);
            notifyAll();
            other.wait();
        }

}

public static void pintMessage(String msg) {
    synchronized (turn) {
     // no changes
      // MyThread.semaphore.notifyAll();
 }
}

see here for mor information https://docs.oracle.com/javase/tutorial/essential/concurrency/

score 0 · Answer 2 · answered Oct 24 '16 at 07:06

0

In your pintMessage no matter what the value of turn is you will print something.

answered Oct 24 '16 at 07:06

Scary Wombat

44,617
6
35
64

Facing issue in trying to synchronize two threads

2 Answers2