Why don't these threads run in sequence?

Question

I have difficulty understanding synchronized and reentrant lock. Here is small program I was experimenting with:

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class Reentr {

    public static void main(String[] args) {
        ExecutorService eService = Executors.newFixedThreadPool(2);
        for (int i = 1; i <= 2; i++) {
            eService.execute(new process());
        }

        try {
            Thread.sleep(1);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

        eService.shutdown();
    }

}

class process implements Runnable {
    int count = 0;

    @Override
    public void run() {

        processTest();
    }

    public synchronized void processTest() {
        try {
            for (int i = 1; i <= 2; i++) {
                count += i;
                System.out.println("count value for " + Thread.currentThread().getName() + " is " + count);
            }
        } finally {
            System.out.println("count for " + Thread.currentThread().getName() + " is " + count);
        }
    }

}

The output was:

count value for pool-1-thread-2 is 1
count value for pool-1-thread-1 is 1
count value for pool-1-thread-2 is 3
count for pool-1-thread-2 is 3
count value for pool-1-thread-1 is 3
count for pool-1-thread-1 is 3

If we see in the output two thread are in the synchronized block. I was under the impression that one thread has to complete the execution of synchronized method, after that other thread will enter the method.

Ideally, I was expecting result something like this

count value for pool-1-thread-1 is 1
count value for pool-1-thread-1 is 3
count for pool-1-thread-1 is 3
count value for pool-1-thread-2 is 1
count value for pool-1-thread-2 is 3
count for pool-1-thread-2 is 3

I have replaced synchronized method with synchronized block and re-entrant locks. However, I still have the same output. What am I missing?

You have new instances of ``process`` each time. There's no contention for their critical sections. — Keith, Aug 17 '15 at 04:20

score 1 · Answer 1 · answered Aug 17 '15 at 04:18

1

When an instance method is declared synchronized, it synchronizes on the object instance. Since you are running with a new process() in each loop iteration, the object instances are different, so the synchronization is meaningless. Try creating a single process object and passing that to both of the threads you start.

When you do, also move the count instance variable into the processTest method. That way it will be thread safe.

answered Aug 17 '15 at 04:18

schtever

3,210
16
25

Thanks for response .. i missed that i was creating new object every time – prred Aug 17 '15 at 04:32

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

The two threads are not synchonizing on the same object.

You have two different instances of process (as an aside; you should always name classes with a Capital letter). The synchronized keyword is equivalent to:

public void processTest() {
  synchronized(this) {
    // etc..
  }
}

If you want one thread to run after the other, they must synchronize on the same object. If you did this, for example:

class process implements Runnable {
  // note that this is static
  private static final Object lock = new Object();

  public void processTest() {
    synchronized(lock) {
      // your code
    }
  }
}

Then your code would have one thread run after the other. Another way to do it would be to pass the lock into the Objects constructor, or the same instance of a Semaphore, etc.

Thanks for reponse .. i missed that i was creating new object every time — prred, Aug 17 '15 at 04:32

Why don't these threads run in sequence?

2 Answers2

The two threads are not synchonizing on the same object.