CODEWITHSUNDEEP
javamultithreadingsynchronized
so_what
so_what

Reputation: 176

Java Concurrency synchronization issue

Can any one explain why the below code will cause race condition as I make the method synchronized but its object level locking, I am following the java concurrency in depth. Please explain because if I use class level it will definitely work, I doubt that the lock occupied by the thread is different for each.

/**
 * 
 */
package lession2.shared.object;

/**
 * @author so_what
 *
 */

class SharedClass {
    private static int sharedData;

    public synchronized   int getSharedData() {
        return sharedData;
    }

    public synchronized   void setSharedData(int sharedData) {

        SharedClass.sharedData = sharedData;
    }

}

//output of the program should be the distinct numbers
public class StaleDataExample extends Thread {
    static  SharedClass s1=new SharedClass();
    static  int counter=0;
    public static  void main(String args[]) throws InterruptedException {

        StaleDataExample t1=new StaleDataExample();
        StaleDataExample t2=new StaleDataExample();
        StaleDataExample t3=new StaleDataExample();
        StaleDataExample t4=new StaleDataExample();
        StaleDataExample t5=new StaleDataExample();
        StaleDataExample t6=new StaleDataExample();
        t1.start();
        t2.start();
        t3.start();
        t4.start();

        t5.start();
        t6.start();
        t1.join();
        t2.join();
        t3.join();
        t4.join();
        t5.join();
        t6.join();
        System.out.println();

    }
    public void run()
    {

        s1.setSharedData(s1.getSharedData()+1); //read->modify->write operation
        System.out.print(s1.getSharedData()+" ");
    }

}

Upvotes: 2

Views: 141

Answers (1)

Mureinik
Mureinik

Reputation: 311028

The issue here is that you aren't incrementing the shared value in an atomic (e.g. synchronized) way.

Let's examine the following line:

s1.setSharedData(s1.getSharedData()+1)

First, you call getSharedData, which is synchronized. You then increment the value, at callsetSharedData` to set the new value. The problem is that the program could context-switch between the get and the set. Consider the following example:

  1. Thread #1 calls getSharedData(), and get 0
  2. Thread #2 calls getSharedData(), and also gets 0
  3. Thread #1 adds 1 to its value, and calls setSharedData(1).
  4. Thread #2 also adds 1 to its value, and calls setSharedData(1), instead of setSharedData(2) that you would expect.

One way to solve such problems is not to allow the class' user to set the value directly, but provide them with a method to atomically increment the value:

class SharedClass {
    private static int sharedData;

    public synchronized int getSharedData() {
        return sharedData;
    }

    public synchronized void incrementSharedData(int amount) {
        sharedData += amount;
    }
}

Upvotes: 2

Related Questions