Race Condition in Java With ReentrantLock

Question

In the following code a ReentrantLock has used to prevent generating odd numbers by the next() method. But the next() method generated odd numbers. But if i change it to nextWithTry it doesn't generate odd numbers. Can anyone explain the reason for this?

class Generator{

    Lock l = new ReentrantLock();
    volatile int c = 0;

    public int next(){

        l.lock();
        c++; c++;
        l.unlock();

        return c;
    }

    public int nextWithTry(){//This method works fine...

        try{
            l.lock();
            c++; c++;
            return c;
        }finally{   
            l.unlock();
        }
    }
}

class W implements Runnable{

    private Generator r;

    public W(Generator r){
        this.r = r;
    }

    @Override
    public void run() {

        int x;

        while(true){
            if(((x = r.next()) % 2) != 0){
                System.out.println(x + " odd number Found");
                break;
            }
        }
    }
}

public class Testing {

    public static void main(String[] args) {

        Generator r = new Generator();

        W w1 = new W(r);

        new Thread(w1).start();
        new Thread(w1).start();
    }
}

Answer 1

What happens when something else increments c between unlock and return?

public int next(){
    //lock, exclusive access
    l.lock();
    //increment, all good
    c++; c++;
    //unlock, another thread can access
    l.unlock();

    //any number of other threads call `next` and can acquire the lock

    //return some random value
    return c;
}

When you use a finally, the lock is only released once the value of c to be returned is already on the stack:

public int nextWithTry() {
    try {
        //lock, exclusive access
        l.lock();
        //increment, all good
        c++; c++;
        //place the value of `c` to be returned on the stack (java passes by value)
        return c;
    } finally {   
        //unlock _after_ the return has been copied
        l.unlock();
    }
}

In fact, the documentation directly recommends using try..finally:

In most cases, the following idiom should be used:

 Lock l = ...;
 l.lock();
 try {
     // access the resource protected by this lock
 } finally {
     l.unlock();
 }

This is to avoid issues like this as well as more serious ones where an Exception causes the Lock not to be unlocked.