Synchronized Method Acquires Second Lock

ID

java.synchronized_calls_synchronized

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

Java

Tags

CWE:833, concurrency, reliability

Description

Reports synchronized methods that contain a synchronized block on a different monitor object. Holding two locks simultaneously creates a lock-ordering dependency: if another code path acquires the same two locks in the opposite order, both threads deadlock permanently.

Rationale

A synchronized method implicitly locks on this. If it also acquires a second lock via synchronized(other), two locks are held at the same time. Deadlock occurs when another thread holds other and tries to enter the same synchronized method (acquiring this).

// Bad -- two locks held simultaneously
public synchronized void transfer(Account other) {
    synchronized (other) {
        // if 'other' also calls transfer(this), deadlock
    }
}

Remediation

Use a consistent lock ordering, a single coarser lock, or java.util.concurrent utilities:

// Good -- consistent ordering by account ID
public void transfer(Account other) {
    Account first = this.id < other.id ? this : other;
    Account second = this.id < other.id ? other : this;
    synchronized (first) {
        synchronized (second) {
            // safe -- always same order
        }
    }
}

References