Concurrency Synchronization Issue

Question

public class Demo {
  public Demo() {
  }

  public static void main(String[] args) {
    Concurrency c = new Concurrency();
    Thread t1 = new Thread(c);
    t1.setName("t1");
    Thread t2 = new Thread(c);
    t2.setName("t2");
    Thread t3 = new Thread(c);
    t3.setName("t3");
    Thread t4 = new Thread(c);
    t4.setName("t4");

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


class Concurrency implements Runnable {
  private String value= new String("I");
  static Integer s =2;

  @Override
  public void run() {
    function();
  }

  public void function() {
    synchronized(s){

    s = s * 5;
    System.out.println("Current thread is " + Thread.currentThread() + s);
    // s.notify();      
  }
}

I wrote the sample program to test the Synchronization. I am getting the following output:

Current thread is Thread[t2,5,main]50
Current thread is Thread[t3,5,main]1250
Current thread is Thread[t4,5,main]1250
Current thread is Thread[t1,5,main]50

This means Synchronization also fails to Synchronize when multiple threads are running, and one more thing I am getting IlleagalMonitorStateException when s.notify() is called as commented above.

Please let me know what exactly it is doing and why Synchronization is failing. Also helo me in fixing this issue.

// After calling s.notify() I am getting the following error.

 Exception in thread "t1" Exception in thread "t2" Exception in thread "t3"   
    java.lang.IllegalMonitorStateException
    Current thread is Thread[t1,5,main]50
    Current thread is Thread[t2,5,main]50
    Current thread is Thread[t3,5,main]250
    Current thread is Thread[t4,5,main]1250
        at java.lang.Object.notify(Native Method)
        at Concurrency.function(Demo.java:42)
        at Concurrency.run(Demo.java:32)
        at java.lang.Thread.run(Unknown Source)
    Exception in thread "t4" java.lang.IllegalMonitorStateException
        at java.lang.Object.notify(Native Method)
        at Concurrency.function(Demo.java:42)
        at Concurrency.run(Demo.java:32)
        at java.lang.Thread.run(Unknown Source)
    java.lang.IllegalMonitorStateException
        at java.lang.Object.notify(Native Method)
        at Concurrency.function(Demo.java:42)
        at Concurrency.run(Demo.java:32)
        at java.lang.Thread.run(Unknown Source)
    java.lang.IllegalMonitorStateException
        at java.lang.Object.notify(Native Method)
        at Concurrency.function(Demo.java:42)
        at Concurrency.run(Demo.java:32)
        at java.lang.Thread.run(Unknown Source)

Answer 1

You should rewrite your function like this.

 public void function()
 {
    synchronized(this){

        s = s * 5;
        System.out.println("Current thread is " + Thread.currentThread() + s);
        this.notify();

    }
 }

Because in this case notify() is not allowed on "s" object. Because "s" is not a thread, its Concurrency Object on which notify() is allowed.

Answer 2

First let's explain your problem:

• Synchronization problem: All threads should open synchronized on the same object, but in your code s gets changed a lot, so the threads open their synchronized on different objects.

• IllegalMonitorStateException: X.notify can only be called when you are in a synchronized(X) { block.

Example:

synchronized (s) { // monitor on "s = 2"
    s = s * 5;     // different object! "s = 10"
    s.notify();    // notify on "s = 10", this is not allowed! 
}

The easiest way to repair this, is to create a monitor object:

class Concurrency implements Runnable {
    private static Integer s = 2; // volatile not needed
    private static final Object monitor = new Object(); // create a monitor object shared by all instances


    @Override
    public void run() {
        function();
    }

    public void function() {
        synchronized (monitor) { // synchronize on monitor object
            s = s * 5;
            monitor.notify(); // notify on the monitor object, although this is not needed since you don't have a wait().
        }
    }
}

Answer 3

According to my tests, this seems to be working fine.

public class Demo
{
    public void function()
    {
        Concurrency c = new Concurrency(this);
        Thread t1 = new Thread(c);
        t1.setName("t1");
        Thread t2 = new Thread(c);
        t2.setName("t2");
        Thread t3 = new Thread(c);
        t3.setName("t3");
        Thread t4 = new Thread(c);
        t4.setName("t4");

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

    public static void main(String[] args)
    {
        Demo demo = new Demo(); //made instance of Demo class
        demo.function();
    }

    static class Concurrency implements Runnable
    {
        private String value = new String("I");
        static volatile Integer s = 2; //made integer 'volatile'

        private Demo demo;

        public Concurrency(Demo demo)
        {
            this.demo = demo; //gets the variable of Demo
        }

        @Override
        public void run()
        {
            function();
        }

        public void function()
        {
            synchronized (demo) //use obtained Demo instance as sync object
            {
                s = s * 5;
                System.out.println("Current thread is " + Thread.currentThread() + s);
                // s.notify(); //I think this needs a wait() call in the same monitor first to work
            }
        }
    }
}

While the order is not determined, the output is in this fashion:

Current thread is Thread[t2,5,main]10
Current thread is Thread[t3,5,main]50
Current thread is Thread[t1,5,main]250
Current thread is Thread[t4,5,main]1250

Answer 4

When you do

s = s * 5;

you are assigning a new object to s, so the syncronization actually works on different objects, and so is inheretly broken. What really happens, behind the scenes, is similar to

s = new Integer(s.intValue() * 5);

What you need is a mutable integer container that is concurrency safe. Luckily there is 'AtomicInteger'.

static final AtomicInteger s = new AtomicInteger(2);

...

s.getAndSet(s.get()*5);