Do Not Use synchronized on a Lock Object
ID |
java.no_synchronize_on_lock_object |
Severity |
low |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
Java |
Tags |
concurrency, reliability |
Description
Reports synchronized blocks that lock on a java.util.concurrent.locks.Lock instance. Lock objects have their own lock()/unlock() protocol and should not be used with intrinsic synchronization.
The monitor is resolved through the symbol table, so the declaration inspected is the one actually in scope at the synchronized statement: a local variable or parameter correctly hides a field of the same name, and a same-named declaration in an unrelated type of the same file is never consulted. The type is matched fully qualified, so a project’s own class named Lock is not mistaken for java.util.concurrent.locks.Lock. Only a bare name (lock) and an explicit this.lock are considered — a monitor reached through another object (other.lock) or returned by a call (getLock()) is not reported, and neither is a name the symbol table cannot attribute to a declaration.
Rationale
Lock objects from java.util.concurrent.locks provide a richer locking API (try-lock, timed lock, interruptible lock, read-write locks) than intrinsic monitors. Using synchronized on a Lock object only acquires the object’s intrinsic monitor, not the Lock itself. This means other code calling lock.lock() will not see the synchronization, creating a subtle concurrency bug.
// Bad -- acquires the intrinsic monitor, not the Lock
Lock lock = new ReentrantLock();
synchronized (lock) {
doWork();
}Remediation
Use the Lock API directly:
// Good -- uses the Lock protocol
Lock lock = new ReentrantLock();
lock.lock();
try {
doWork();
} finally {
lock.unlock();
}