Futile synchronization on field¶
ID: java/unsafe-sync-on-fieldKind: problemSecurity severity: Severity: errorPrecision: mediumTags: - quality - reliability - correctness - concurrency - language-features - external/cwe/cwe-662Query 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 storedin 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 thesetField method.
publicclassA{privateObjectfield;publicvoidsetField(Objecto){synchronized(field){// BAD: synchronize on the field to be updatedfield=o;// ... more code ...}}}
In class B, the recommended approach is shown, where synchronization takes place on a separate lock object.
publicclassB{privatefinalObjectlock=newObject();privateObjectfield;publicvoidsetField(Objecto){synchronized(lock){// GOOD: synchronize on a separate lock objectfield=o;// ... more code ...}}}
References¶
Java Language Specification:The synchronized Statement,synchronized Methods.
The Java Tutorials:Lock Objects.
Common Weakness Enumeration:CWE-662.