Synchronous method doesn't work as expected
I have a variable that is shared by two threads. These two threads will perform some operations on it. I don't know why the result of sharedVar is different every time the program is executed.
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 not
}
}
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 end result of sharedVar is sometimes 13735, 12508, or 18793; but never the 20,000 that I expect. Another interesting thing about the program is time = 1000. I always get 2000 as the end result.
Can anyone explain this phenomenon?
source to share
A synchronized method protects the resource this
, which means 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 the method is called addOne
. This means that this
for is mt1.addOne
not the same as this
for mt2.addOne
, and therefore you do not have a sync share.
Try changing your yout code addOne
to:
private void addOne()
{
synchronized(MyThread.class)
{
for (int i = 0; i < times; ++i)
{
Main.sharedVar ++;
}
}
}
And you will observe the expected behavior. As you can see from the notes below, it is better to use a different object than MyThread.class
for synchronization because class objects are accessible from many points and it is easy that other code might try to synchronize using the same object.
source to share
When you use synchronized
for a non-static method, you are using the current object as the monitor.
When you use synchronized
for a static method, you are using the current class object ( ClassName.class
static field) as a monitor.
In your case, you are using synchronized
for a Thread object (2 different instances), so two different threads will modify your static field sharedVar
at the same time.
You can fix this in different ways.
Move the method addOne
to Main
and do it static
.
private static synchronized void addOne(int times)
{
for (int i = 0; i < times; ++i)
{
sharedVar++;
}
}
Or you can create a class named sharedVar
with field private int var;
and method synchronized void addOne(int times)
and pass one instance sharedVar
to your protectors.
public static void main(String[] args)
{
SharedVar var = new SharedVar();
MyThread mt1 = new MyThread(var);
MyThread mt2 = new MyThread(var);
mt1.start();
mt2.start();
try
{
// wait for the threads
mt1.join();
mt2.join();
}
catch (InterruptedException e1)
{
e1.printStackTrace();
}
System.out.println(var.getVar()); // I expect this value to be 20000, but it not
}
But if you only need to change one integer in multiple threads, you can use classes from java.til.concurrent.*
e.g. AtomicLong
or AtomicInteger
.
source to share
When a thread is about to execute a " synchronized " instance method , it will lock the Object (more precisely, lock this object monitor).
So, in your case, Thread mt1 acquires the lock on the mt1 object, and the Thread mt2 acquires the lock on the mt2 object, and they do not block each other since the two threads are working with two different locks.
And when two threads modify a shared variable at the same time (not synchronized), the result is unpredictable.
Ok about the case of 1000, for smaller inputs interleaving can lead to the correct result (fortunately).
Sol: remove synchronized keyword from addOne method and make sharedVal as type ' AtomicInteger
source to share
Attach the stream right after the start method. From this thread-1 will start and go to dead state after thread-2 starts and goes dead. Therefore, it will always print your expected output.
Modify the code as shown below: -
public class Main{
public static int sharedVar = 0;
public static void main(String[] args)
{
MyThread mt1 = new MyThread();
MyThread mt2 = new MyThread();
try
{
mt1.start();
mt1.join();
mt2.start();
mt2.join();
}
catch (InterruptedException e1)
{
e1.printStackTrace();
}
System.out.println(sharedVar);
}
}
class MyThread extends Thread
{
private int times = 1000000;
private synchronized void addOne()
{
for (int i = 0; i < times; ++i)
{
Main.sharedVar++;
}
}
@Override
public void run()
{
addOne();
}
}
source to share