Why java thread behave so differently if they shouldnt in this scenario?

Question

I have a thread sleep problem. Inside the thread run method i have a synchronized block, and a sleep time. Each thread increments or decrements the shared class "value" in 5 units, and then sleeps.

public class borr {

    public static void main(String[] args) {

        int times=5;
        int sleeptime=1000;
        int initial=50;
        Shared shared = new Shared(initial);

        ThreadClass tIncrement = new ThreadClass(shared,times,sleeptime,true);
        ThreadClass tDecrement = new ThreadClass(shared,times,sleeptime,false);
        tIncrement.start();
        tDecrement.start();
    }
}

---

class Shared{

    int  value=0;

    public Shared(int value) {
        super();
        this.value = value;
    }

    public int getValue() {
        return value;
    }

    public void setValue(int value) {
        this.value = value;
    }
}

---

class ThreadClass extends Thread{

    Shared shared;
    int times=0;
    int sleeptime=0;
    boolean inc;

    public ThreadClass(Shared shared, int times, int sleeptime, boolean inc) {
        super();
        this.shared = shared;
        this.times = times;
        this.sleeptime = sleeptime;
        this.inc = inc;
    }

    public void run() {

        int aux;

        if(inc) {
            for(int i=0;i<times;i++) {
                synchronized(shared) {
                    aux=shared.getValue()+1;
                    shared.setValue(aux);
                    System.out.println("Increment, new value"+shared.getValue());

                    try {
                        Thread.sleep(sleeptime);
                    }catch(Exception e) {
                        e.printStackTrace();
                    }
                }
            }   
        }
        else {
            for(int i=0;i<times;i++) {
                synchronized(shared) {
                    aux=shared.getValue()-1;
                    shared.setValue(aux);
                    System.out.println("Decrement, new value"+shared.getValue());

                    try {
                        Thread.sleep(sleeptime);
                    }catch(Exception e) {
                        e.printStackTrace();
                    }
                }
            }   
        }
    }
}

But if I move the Thread.sleep out of the synchronized block, like this, the output is increment, decrement, increment, decrement. When it stops sleeping and starts a new iteration of the loop, shouldn't the other thread try to enter? instead, it continues looping until that thread is finished:

for(int i=0;i<times;i++) {
    synchronized(shared) {
        aux=shared.getValue()-1;
        shared.setValue(aux);
        System.out.println("Decrement, new value"+shared.getValue());
    }

    try {
        Thread.sleep(sleeptime);
    }catch(Exception e) {
        e.printStackTrace();
    }
}   

Answer 1

This is bad:

for(...) {
    synchronized(some_lock_object) {
        ...
    }
}

The reason it's bad is, Once some thread, A, gets into that loop, then every time it unlocks the lock, The very next thing it does is to lock the lock again.

If the loop body takes any significant amount of time to execute, then any other thread, B, that's waiting for the lock will be put into a wait state by the operating system. Each time thread A releases the lock, thread B will start to wake up, but thread A will be able to re-acquire it before thread B gets a chance.

This is a classic example of starvation.

One way around the problem would be to use a ReentrantLock with a fair ordering policy instead of using a synchronized block. When threads compete for a fair lock, the winner always is the one that's been waiting the longest.

But, fair locks are expensive to implement. A far better solution is to always keep the body of any synchronized block as short and as sweet as possible. Usually, a thread should keep a lock locked for no longer than it takes to assign a small number of fields in some object.

Answer 2

But if i move the Thread.sleep out of the synchronized block, like this, the output is increment, decrement, increment, decrement. The sleep is still inside each iteration of the loop so, shouldnt the result be the same in both cases?:

when it stops sleeping and starts a new iteration of the loop, shouldn't the other thread try to enter.

They both try to enter.

And the other one is already in a wait status (i.e. not actively running) because it tried to enter before. Whereas the thread that has just released the lock can run on and get the now uncontested lock right back.

This is a race condition. When both threads want the lock at the same time, the system is free to choose one. It seems it picks the one that a few instructions ago just released it. Maybe you can change this by yield()ing. Maybe not. But either way, it is not specified/deterministic/fair. If you care about execution order, you need to explicitly schedule things yourself.

Answer 3

In variant A you use two threads that ...

• repeat 5 times
  • enter a sync block
    • increment
    • wait 1 second
• repeat 5 times
  • enter a sync block
    • decrement
    • wait 1 second

In variant B you use two threads that ...

• repeat 5 times
  • enter a sync block
    • increment
  • wait 1 second
• repeat 5 times
  • enter a sync block
    • decrement
  • wait 1 second

In variant A both threads are active (= stay in a sync block) all the time.

In variant B both threads are sleeping most of the time.

As there is absolutely no guarantee which threads are executed next, it is not surprising that variant A and B behave so differently. While in A both threads could - in theory - be active in parallel, the second thread has not much chance to be active as not being in a synchronization context does not guarantee that a context switch is performed at that moment (and another thread is run). In variant B that is completely different: As both threads sleep most of the time, the runtime environment has no other chance as running another thread while one is sleeping. A sleep will trigger switching to another thread as the VM tries to make the best of existing CPU resources.

Nevertheless: The result AFTER both threads have been run will be exactly the same. This is the only determinism you can rely on. Everything else depends on specific implementation details how the VM will handle threads and synchronizations blocks and can even vary from OS to OS or one implementation of a VM to another.