71 lines
2.1 KiB
Plaintext
71 lines
2.1 KiB
Plaintext
== Why is this an issue?
|
|
|
|
Synchronizing on a class field synchronizes not on the field itself, but on the object assigned to it. So synchronizing on a non-``++final++`` field makes it possible for the field's value to change while a thread is in a block synchronized on the old value. That would allow a second thread, synchronized on the new value, to enter the block at the same time.
|
|
|
|
The story is very similar for synchronizing on parameters; two different threads running the method in parallel could pass two different object instances in to the method as parameters, completely undermining the synchronization.
|
|
|
|
=== Noncompliant code example
|
|
|
|
[source,java,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
private String color = "red";
|
|
|
|
private void doSomething(){
|
|
synchronized(color) { // Noncompliant; lock is actually on object instance "red" referred to by the color variable
|
|
//...
|
|
color = "green"; // other threads now allowed into this block
|
|
// ...
|
|
}
|
|
synchronized(new Object()) { // Noncompliant this is a no-op.
|
|
// ...
|
|
}
|
|
}
|
|
----
|
|
|
|
=== Compliant solution
|
|
|
|
[source,java,diff-id=1,diff-type=compliant]
|
|
----
|
|
private String color = "red";
|
|
private final Object lockObj = new Object();
|
|
|
|
private void doSomething(){
|
|
synchronized(lockObj) {
|
|
//...
|
|
color = "green";
|
|
// ...
|
|
}
|
|
}
|
|
----
|
|
|
|
== Resources
|
|
|
|
* CWE - https://cwe.mitre.org/data/definitions/412[CWE-412 - Unrestricted Externally Accessible Lock]
|
|
* CWE - https://cwe.mitre.org/data/definitions/413[CWE-413 - Improper Resource Locking]
|
|
* https://wiki.sei.cmu.edu/confluence/x/djdGBQ[CERT, LCK00-J.] - Use private final lock objects to synchronize classes that may interact with untrusted code
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
"xxx" is not "private final", and should not be used for synchronization.
|
|
|
|
"xxx" is a method parameter, and should not be used for synchronization.
|
|
|
|
Synchronizing on a new instance is a no-op.
|
|
|
|
=== Highlighting
|
|
|
|
``++synchronize xxx++``
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
include::../comments-and-links.adoc[]
|
|
|
|
endif::env-github,rspecator-view[] |