Futile synchronization on field¶

ID: java/unsafe-sync-on-field
Kind: problem
Security severity: 
Severity: error
Precision: medium
Tags:
   - quality
   - reliability
   - correctness
   - concurrency
   - language-features
   - external/cwe/cwe-662
Query suites:
   - java-security-and-quality.qls

Click to see the query in the CodeQL repository

A block of code that synchronizes on a field and updates that field while the lock is held is unlikely to provide the desired thread safety. Such a synchronized block does not prevent multiple unsynchronized assignments to that field because it obtains a lock on the object stored in the field rather than the field itself.

Recommendation¶

Instead of synchronizing on the field itself, consider synchronizing on a separate lock object when you want to avoid simultaneous updates to the field. You can do this by declaring a synchronized method and using it for any field updates.

Example¶

In the following example, in class A, synchronization takes place on the field that is updated in the body of the setField method.

public class A {
    private Object field;  
    
    public void setField(Object o){
        synchronized (field){    // BAD: synchronize on the field to be updated
            field = o;
            // ... more code ...          
        }
    }
}

In class B, the recommended approach is shown, where synchronization takes place on a separate lock object.

public class B {
   private final Object lock = new Object();
   private Object field;

   public void setField(Object o){
       synchronized (lock){      // GOOD: synchronize on a separate lock object
           field = o;
           // ... more code ...
       }
   }
}

References¶

Java Language Specification: The synchronized Statement, synchronized Methods.
The Java Tutorials: Lock Objects.
Common Weakness Enumeration: CWE-662.

GeezerVison

Futile synchronization on field¶

Recommendation¶

Example¶

References¶