java - Why is synchronized not working properly?

Question

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 1

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.