Hemlata Hemlata - 1 month ago 10
Java Question

Why is synchronized not working properly?

Here's my code:

private int count = 0;

public synchronized void increment() {
count++;
}

public void doWork() throws InterruptedException {

Thread t1 = new Thread(new Runnable() {
public void run() {
for (int i = 0; i < 5; i++) {
increment();
System.out.println(count+" "+Thread.currentThread().getName());
}}});

Thread t2 = new Thread(new Runnable() {
public void run() {
for (int i = 0; i < 5; i++) {
increment();
System.out.println(count+" "+Thread.currentThread().getName());
}}});

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


Here's my output:

2 Thread-1
2 Thread-0
3 Thread-1
5 Thread-1
6 Thread-1
4 Thread-0
8 Thread-0
9 Thread-0
7 Thread-1
10 Thread-0


My understanding is that
increment
is
synchronized
. So, it should first
increment
one number and then release the
lock
and then give the
lock
to the thread
t1
or
t2
. So, it should
increment
one number at a time, right?

But why is my code
incrementing
two or three numbers at a time? Am I doing something wrong (I'm a newbie)?

Answer

While count++; indeed is synchronized System.out.println(count+" "+Thread.currentThread().getName()); is not, but it access to count variable.

Even if you synchronize access, it won't help you because next scenario will be still possible:

  • Thread 1 increment
  • Thread 2 increment
  • Thread 1 print value 2
  • Thread 2 print value 2

To fix this issue you need to increment and print in same synchronized section. For example you can put System.out.println(count+" "+Thread.currentThread().getName()); into increment method.