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?
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
.
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With