Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is synchronized locking a Reentrantlock, or only its object?

The normal pattern with ReentrantLock and lock()/unlock() is like this:

lck.lock();
try {
    // ...
}
finally {
    lck.unlock();
}

Can this be refactored to

synchronized(lck) {
    // ...
}

?
And why?

like image 612
fbenoit Avatar asked Oct 27 '16 17:10

fbenoit


2 Answers

These are different things. synchronized is built into the language and can be used with any object. What it does is lock its intrinsic lock. Every single object has one. As it's a built-in mechanism, you don't need a try-finally block—the lock is always unlocked when the control exits the synchronized block. So as long as your code actually exits that block, the lock will be unlocked.

ReentrantLock is a special class. It locks some special internal object, that is probably implementation-specific. It does not lock its intrinsic lock. You could, of course, lock that one too—but it doesn't normally make any sense. This code will almost certainly deadlock, for example:

final ReentrantLock lock = new ReentrantLock();
new Thread(() -> {
    lock.lock();
    try {
        System.out.println("Thread 1 locked the lock");
        try { Thread.sleep(100); } catch (Exception ex) {}
        synchronized (lock) {
            System.out.println("Thread 1 locked lock's intrinsic lock");
        }
    } finally {
        lock.unlock();
    }
}).start();
new Thread(() -> {
    synchronized (lock) {
        System.out.println("Thread 2 locked lock's intrinsic lock");
        try { Thread.sleep(200); } catch (Exception ex) {}
        lock.lock();
        try {
            System.out.println("Thread 2 locked the lock");
        } finally {
            lock.unlock();
        }
    }
}).start();

It deadlocks because two threads lock two different things in different order.

It certainly feels like ReentrantLock does almost the same thing as synchronized. It works similarly, but synchronized is both more convenient and less powerful. So unless you need any features of ReentrantLock, like interruptible lock attempts or lock time-outs, you should stick with synchronized for the purpose of reentrant locking, and use any objects for that. Simple private final Object lock = new Object() will do just fine. Note that final will prevent confusing things that could happen if you change that object at some moment; some IDEs will issue a warning if you omit final.

like image 76
Sergei Tachenov Avatar answered Sep 28 '22 16:09

Sergei Tachenov


I assume that you are aware of differences in explicit and implicit locking provided by Lock and synchronized respectively.

I believe you are looking for a reason saying what's wrong in using instances of class implementing Lock interfaces inside synchronized block as in synchronized(lock).

Could it be refactored? Yes.

But should you be doing that? Not with instances of classes implementing Lock interface

Why? - Well.

It is all right if you just use lock only inside synchronized however you leave the possibility to other developers to misuse the code say for e.g. what if someone tomorrow tries calling Lock methods inside synchronized(lock) something like below.

Lock lock = new ReentrantLock();
synchronized(lock){ //You write this
    // lock.lock();  // I am not taking a lock here
    System.out.println("See emily play");
    ...
    ...
    ... // after 100 lines of code 
    //callAnotherMethod(lock); //Someone else does this
    lock.unlock();   //Someone else does this
}

The above code is horrible but to give you an example, in the above example if you do not call lock(), then you end up with IllegalMonitorStateException. If you do call (uncomment above) lock.lock() it makes no difference.

Not to mention the callAnotherMethod(lock) where you are passing the lock instance and what sort of unexpected behaviours it can introduce.

Keep in mind that is one such example.

Bottom line, if it works correctly by any chance, it is just wasting resources and time and would serve no advantage/purpose. More importantly there is no guarantee that it would not introduce regressions in the future. And if there would be any such regressions, you may end up wasting significant amount of time because of misuse of concepts.

Softwares are always designed with Open-Close principle. You would be writing the code that would violate it very clearly.

In case if you do want to use fine grained locks using synchronized then you can make use of the below

Object obj1 = new Object();
Object obj2 = new Object();

public void doSomething(){
    synchronised(obj1){
    ...
    }
}

public void doSomethingMore(){
    synchronised(obj2){
    ...
    }
}

But then again, I don't see any reason why you would not use multiple lock instances to achieve the above.

like image 25
Kamal Avatar answered Sep 28 '22 15:09

Kamal