Multiple Threads accessing instance method from different Instances should cause a race condition?

Question

I am trying to understand Synchornized in Java. I understood if I have access a synchronized method on same object from 2 different Threads, only one will be able to access at a time.

But I think if the same method is being called on 2 different instances, Both Objects should be able to access the method parallel. Which would cause race condition if accessing/modifying a static member variable from the method. But I am not able to see the race condition happening in below code.

Could someone please explain whats wrong with the code or my understanding.

For reference code is accessible at : http://ideone.com/wo6h4R

class MyClass
{
   public static int count=0;

   public  int getCount() 
   {

        System.out.println("Inside getcount()");
        return count;
   }

   public synchronized void incrementCount() 
   {
        count=count+1;
   }


}

class Ideone
{
    public static void main(String[] args) throws InterruptedException {
        final MyClass test1 = new MyClass();
        final MyClass test2 = new MyClass();
        Thread t1 = new Thread() { 
                                public void run()  
                                { 
                                    int k=0;
                                    while (k++<50000000)
                                    {
                                          test1.incrementCount();

                                    } 
                                }
                                };

    Thread t2 = new Thread() { 
                                public void run() 
                                { 
                                    int l=0;
                                    while (l++<50000000)
                                    {
                                            test2.incrementCount(); 

                                    } 
                                }
                                };

    t1.start();

    t2.start();
    t1.join();
    t2.join();
    //System.out.println(t2.getState());
    int x=500000000+500000000;
    System.out.println(x);
    System.out.println("count = " + MyClass.count);

}

}

@PM77-1 Because `synchronized` keyword is not a magic wand. It merely synchronizes on an object that is an instance of the class. Two different objects means two different locks. Synchronizing on different locks means not synchronizing at all. The OP is right. This is an instance of using it incorrectly. — Santa, Jun 25 '14 at 17:46

score 7 · Accepted Answer · answered Jun 25 '14 at 17:43

7

You're right that the race condition exists. But the racy operations are so quick that they're unlikely to happen -- and the synchronized keywords are likely providing synchronization "help" that, while not required by the JLS, hide the races.

If you want to make it a bit more obvious, you can "spell out" the count = count + 1 code and put in a sleep:

public synchronized void incrementCount() 
{
    int tmp = count + 1;
    try {
        Thread.sleep(500);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
    count=tmp;
}

That should show the races more easily. (My handling of the interrupted exception is not good for production code, btw; but it's good enough for small test apps like this.)

The lesson learned here is: race conditions can be really hard to catch through testing, so it's best to really understand the code and prove to yourself that it's right.

answered Jun 25 '14 at 17:43

yshavit

42,327
7
87
124

3

A sleep of `random(400)+100` would be much better, but the answer is to the point. Also, worth noting that without the sleep it's almost impossible for this to happen on single-core machines. – Ordous Jun 25 '14 at 17:46
@Ordous Good points on both accounts! I'll keep my post as is for the sake of simplicity, but you're definitely right. (I didn't mention the single-core machine bit because do those even exist anymore? ;) ) – yshavit Jun 25 '14 at 17:48
I dont really follow why it's difficult to produce race condition with the above code, as When I run the two threads on same Object with removing synchronized from the method I do see unguaranteed count. Isnt 2 threads calling an un-synchronized method on same Object is equivalent to 2 threads calling synchronized method from Different Instances ?? – Kumar Alok Jun 25 '14 at 17:54
@KumarAlok By the language spec, yes. But the implementation is probably being less clever than it's allowed to be, and is therefore issuing machine code instructions to synchronize memory visibility or such, in ways that it doesn't _have_ to in this instance, but usually would when it sees `synchronized`. – yshavit Jun 25 '14 at 17:57
@yshavit I tried with Sleep and Random Sleep.. Still not able to see race ? – Kumar Alok Jun 25 '14 at 17:59
@yshavit But wouldn't that mean that its synchronizing the method for all the instances ? – Kumar Alok Jun 25 '14 at 18:12
@KumarAlok Just ran **your** code on my machine and got 89008114 as final count. Are you sure you have full concurrency support on? (I.e. multi-core machine, JVM allowed to use more than one OS thread, etc) – Ordous Jun 25 '14 at 18:13
@KumarAlok (and as expected, making the method static synchronized fixes it to be 100000000) – Ordous Jun 25 '14 at 18:14
@Ordous I dont want to fix it :) And its good if you are able to see race condition. I am running this code on ideone.com and it was giving me race condition with 2 threads accessing an un-synchronized method. I will download eclipse and run it on my machine then. And keep you posted. – Kumar Alok Jun 25 '14 at 18:19
I tried running the code on eclipse and it caused race condition in 1 go itself :) Thanks @Ordous – Kumar Alok Jun 25 '14 at 18:57
@KumarAlok It could well be that ideone is playing tricks with threading so that no one process can spam its servers with threads. – yshavit Jun 25 '14 at 19:08
@yshavit Thats right, But again as I mentioned with Unsynchronized method also it should be the same case then. Anyways I have successfully simulated race, Thank you so much for you help :) – Kumar Alok Jun 27 '14 at 14:36

score 0 · Answer 2 · answered Jun 25 '14 at 17:48

Since syncrhonized methods actually synchronize on this different instance methods will lock on different objects and therefore you will get race conditions since they don't block each other.

You probably have to make your own lock object and lock on that.

   class MyClass
    {
       public static int count=0;
           //this is what you lock on
       private static Object lock = new Object();

       public  int getCount() 
       {
           synchronized(lock){

                System.out.println("Inside getcount()");
                return count;
           }
       }

       public void incrementCount() 
       {
           synchronized(lock){
               count = count+1;
           }

       }
          //etc

Now when you run your main, this gets printed out:

1000000000
count = 100000000

score 0 · Answer 3 · edited May 23 '17 at 12:29

0

Here's the relevant section of the Java specification:

"A synchronized method acquires a monitor (§17.1) before it executes. For a class (static) method, the monitor associated with the Class object for the method's class is used. For an instance method, the monitor associated with this (the object for which the method was invoked) is used."

However I fail to see where the MyClass' instances are actually incrementing "count" so what exactly are you expecting to show as a race condition?

(Taken originally from this answer)

edited May 23 '17 at 12:29

Community

1
1

answered Jun 25 '14 at 18:02

Acapulco

3,373
8
38
51

@Ordous Ah! I see that now. – Acapulco Jun 25 '14 at 18:05

Multiple Threads accessing instance method from different Instances should cause a race condition?

3 Answers3