Why are my two simple Java threads not getting interleaved access?

Viewed 122

I have read posts on thread interleaving and Java thread scheduling, but I don't understand why my two simple Java threads, running against an exclusive Resource, don't to some degree get interleaved access to the Resource.

In the code below, I have a Resource class, with its sole static access() method containing a synchronized block. I simply create and start two MyThread instances in main, each thread with looping repeated calls to Resource.access(). Every access sleeps for 1 sec, then frees the Resource.

public class X {
    public static void main(String[] args) {
        System.out.println("Creating threads");

        Thread t1 = (new Thread(new MyThread()));
        System.out.println(String.format("Thread t1 id = %d",
            t1.getId()));

        Thread t2 = (new Thread(new MyThread()));
        System.out.println(String.format("Thread t2 id = %d",
            t2.getId()));

        System.out.println("Starting threads");
        t1.start();
        t2.start();
    }
}

class MyThread implements Runnable {    
    @Override
    public void run () {
        for (int i = 0; i <= 5; i++) {
            Resource.access();
        }
        System.out.println(String.format("Thread %d completed",
            Thread.currentThread().getId()));
    }
}

class Resource {
    private static Object monitor = new Object();

    public static void access() {
        synchronized (monitor) {
            System.out.println(String.format(
                "Thread %d accessing resource", 
                Thread.currentThread().getId()));
            try {
                Thread.sleep(1000);
            } catch (InterruptedException ie) {
                ie.printStackTrace();
                System.exit(1);
            }
            System.out.println(String.format(
                "Thread %d leaving resource", 
                Thread.currentThread().getId()));
        }
    } 
}

No matter what I try, however, the loop of the MyThread instance started first (t1 in the above example) completes all accesses to the Resource before the other MyThread instance (t2 in the example above) gets any access to the Resource. Here is the output:

Creating threads
Thread t1 id = 12
Thread t2 id = 13
Starting threads
Thread 12 accessing resource
Thread 12 leaving resource
Thread 12 accessing resource
Thread 12 leaving resource
Thread 12 accessing resource
Thread 12 leaving resource
Thread 12 accessing resource
Thread 12 leaving resource
Thread 12 accessing resource
Thread 12 leaving resource
Thread 12 accessing resource
Thread 12 leaving resource
Thread 13 accessing resource
Thread 12 completed
Thread 13 leaving resource
Thread 13 accessing resource
Thread 13 leaving resource
Thread 13 accessing resource
Thread 13 leaving resource
Thread 13 accessing resource
Thread 13 leaving resource
Thread 13 accessing resource
Thread 13 leaving resource
Thread 13 accessing resource
Thread 13 leaving resource
Thread 13 completed

I read that a Java thread at normal priority may get up to a 1-second time slice, so I would expect Thread 13 above (t2) to have interleaved access to the Resource, not necessarily one for one, but some interleave.

2 Answers

Move your sleep statement outside the synchronized block to give the other thread a chance to access it. Otherwise you are relying on a very small window of opportunity for one thread to access the method when the monitor is released.

Another way is to leave your sleep where it is but change the duration from 1000 to 100 milliseconds. Then increase your loop from 5 to 5000.

The bottom line is that thread resource competition does not always manifest itself as one might expect. Threads don't always cooperate to demonstrate successful interaction.

class Resource {
    private static Object monitor = new Object();
    
    public static void access() {
        synchronized (monitor) {
            System.out.println(
                    String.format("Thread %d accessing resource",
                            Thread.currentThread().getId()));
            System.out.println(
                    String.format("Thread %d leaving resource",
                            Thread.currentThread().getId()));
        }
        try {
            Thread.sleep(1000);
        } catch (InterruptedException ie) {
            ie.printStackTrace();
            System.exit(1);
        }
    }

TL;DR : for performance reasons, synchronized() in Java doesn't use fair locks. Replace your synchronized() block with ReentrantLock and pass true to the "fair" argument to the constructor of ReentrantLock. But see the explanation below.

This is about efficiency. Here is what's happening in your threads :

  • Both threads start. One of them takes the lock. The other goes to sleep as it's waiting on the lock. It can go either way, let's call the one that takes the lock Thread A.
  • Thread A uses the resource for 1 second (symbolized with your sleep() inside the lock)
  • Thread A releases the lock. This, in itself, does not particularly trigger a context switch : Thread A will continue to run. Thread B is still sleeping at this point.
  • CHANCE here is where you'd expect Thread B to take the lock but it's sleeping, see below.
  • Thread A has very few instructions to go until its next check of the lock. It executes them, and since Thread B is still sleeping, it finds the lock free and takes it again.
  • This repeats until Thread A is done.

I can see you would expect Thread B to wake up and take the lock in CHANCE, but it's not that simple. For this to happen the thread has to be running to try and take the lock. But it's sleeping, for good reason : it's been put to sleep waiting for a lock. Thread B does not have a core idle ready to try and take the lock as soon as it's released because that would be a waste of resources (even on multicore CPUs, where other cores have other things to do and even if they don't they are shut down to conserve power).
Now in CHANCE it is technically possible that Thread A just happens to be pre-empted in these very few instructions and Thread B happens to be woken up at that time and go forward. It's just very unlikely because Thread A has so little to do until it can lock again.

That being said, your problem is a (slightly) common problem that other people had, and there is a tool to solve it : fair locks. A fair lock has a strategy that makes sure each thread has an equal chance of grabbing the lock, specifically to address the kind of issue you are seeing here. In Java, ReentrantLock implements this if you pass true to its constructor.

Note : using fair locks comes with a small performance penalty, because the lock has code to implement the fairness. When a thread takes the lock, the lock has to check what other threads are waiting to make sure it's not always the same thread taking the lock. That is why synchronized() does not use fair locks by default : performance and contention issues with locks are a lot more common than fairness problems.
Note : fair locks also don't guarantee ordering in a case like yours. They just stack the odds in favor of the longer waiting thread, and guarantee that eventually it will get a turn. See the doc on ReentrantLock for details.

Related