java - Synchronized method does not work as expected

Question

I have a variable which is shared by two threads. The two threads will do some operations on it. I don't know why the result of sharedVar is different every time I execute the program.

public class Main
{
    public static int sharedVar = 0;
    public static void main(String[] args) 
    {
        MyThread mt1 = new MyThread();
        MyThread mt2 = new MyThread();
        mt1.start();
        mt2.start();

        try
        {
            // wait for the threads
            mt1.join();
            mt2.join();
        }
        catch (InterruptedException e1)
        {
            e1.printStackTrace();
        }

        System.out.println(sharedInt); // I expect this value to be 20000, but it's not
    }
}

The following is the class "MyThread"

public class MyThread extends Thread
{
    private int times = 10000;
    private synchronized void addOne()
    {
        for (int i = 0; i < times; ++i)
        {
            Main.sharedVar ++;
        }
    }

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

The final result of sharedVar sometimes are 13735, 12508, or 18793; but never 20000, which is the result I expect. Another interesting thing about the program is when times=1000. I always get 2000 as the final result.

Can anyone explain this phenomenon?

Answer 1

A synchronized method protects the resource this that means that your code is equivalent to:

private void addOne()
{
    synchronized(this)
    {
        for (int i = 0; i < times; ++i)
        {
            Main.sharedVar ++;
        }
    }
}

But you have 2 objects for which addOne method is called. That means this for mt1.addOne is not the same than this for mt2.addOne and therefore you don't have a common resource of synchronization.

Try changing yout addOne code to:

private void addOne()
{
    synchronized(MyThread.class)
    {
        for (int i = 0; i < times; ++i)
        {
            Main.sharedVar ++;
        }
    }
}

And you will observe the expected behaviour. As the comments below suggest, it is better to use a different object than MyThread.class for synchronization since class objects are accesible from many points and it is easy that other code may try to synchronize using the same object.