This is an issue when using synchronized blocks on objects from the java.util.concurrent package, such as ReentrantLock, Semaphore, CountDownLatch, CyclicBarrier, BlockingQueue, or atomic types like AtomicInteger.

Why is this an issue?

Concurrent programming libraries provide their own high-level synchronization mechanisms. These are designed to be more flexible and powerful than the language’s basic object locking mechanism.

When you use the basic locking mechanism on such an object, you are acquiring a lock that is built into the object itself at the language runtime level. However, this built-in lock is completely separate from the object’s own synchronization protocol. The two mechanisms do not interact with each other at all.

For example:

• Using basic locking on a reentrant lock object does NOT call the object’s lock acquisition or release methods
• Using basic locking on a semaphore object does NOT call the object’s acquire or release methods
• Using basic locking on a countdown latch object does NOT call the object’s await or countdown methods
• Using basic locking on an atomic variable does NOT use its compare-and-swap operations

This means that threads using the basic locking mechanism on these objects will not coordinate with threads using the object’s proper API methods. Different threads might think they have exclusive access when they actually don’t, leading to race conditions and data corruption.

This pattern typically indicates a misunderstanding of how concurrent library classes work. The developer likely intended to use the object’s own synchronization mechanism but mistakenly used the basic locking mechanism instead.

In Java, this refers to classes from the java.util.concurrent package (including java.util.concurrent.locks and java.util.concurrent.atomic) and using the synchronized keyword. Specific examples include using synchronized(reentrantLock) instead of calling lock()/unlock() on a ReentrantLock, or using synchronized(semaphore) instead of calling acquire()/release() on a Semaphore, or using synchronized(atomicInt) instead of calling compareAndSet()/getAndUpdate() on an AtomicInteger.

What is the potential impact?

When language-level locking mechanisms are used on concurrent data structures that implement their own internal synchronization, the code fails to provide the expected thread-safety guarantees:

• Race conditions: Multiple threads may access shared resources simultaneously, even though the code appears to prevent this
• Data corruption: Concurrent modifications to shared data can lead to inconsistent state
• Logic errors: The application may behave incorrectly in multi-threaded scenarios, with bugs that are difficult to reproduce and diagnose
• False sense of security: The presence of explicit locking blocks may give developers and reviewers false confidence that the code is thread-safe

These issues are particularly dangerous because they often only manifest under specific timing conditions, making them hard to detect during testing.

How to fix it

Replace the synchronized block with the correct API methods for the specific java.util.concurrent class you’re using. Always use try-finally blocks when acquiring locks to ensure they are released even if an exception occurs.

Code examples

Noncompliant code example

private final ReentrantLock lock = new ReentrantLock();

public void doWork() {
    synchronized (lock) {  // Noncompliant
        criticalSection();
    }
}

Compliant solution

private final ReentrantLock lock = new ReentrantLock();

public void doWork() {
    lock.lock();
    try {
        criticalSection();
    } finally {
        lock.unlock();
    }
}

Resources

Standards

• CERT, LCK03-J. - Do not synchronize on the intrinsic locks of high-level concurrency objects

Documentation

• Oracle Java Documentation - java.util.concurrent
• Oracle Java Documentation - Lock

Related rules

• {rule:java:S2445} - Blocks should be synchronized on "private final" fields