LCK04-J
3.5.2Compliant Solution (Collection Lock Object)
This compliant solution synchronizes on the map view instead of the set view.
// map has package-private accessibility final Map<Integer, String> map =
Collections.synchronizedMap(new HashMap<Integer, String>()); private final Set<Integer> set = map.keySet();
public void doSomething() {
synchronized(map) { // Synchronize on map, not set for (Integer k : set) {
// ...
}
}
}
This code is compliant because the map’s underlying structure cannot be changed when an iteration is in progress.
3.5.3Risk Assessment
Synchronizing on a collection view instead of the collection object can cause nondeterministic behavior.
Guideline |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
LCK04-J |
low |
probable |
medium |
P8 |
L2 |
3.5.4References
[Sun 2009b] |
Class Collections |
[Sun 2008a] |
Wrapper Implementations |
CMU/SEI-2010-TR-015 | 58
LCK05-J
3.6LCK05-J. Synchronize access to static fields that may be modified by untrusted code
Methods that can be invoked from untrusted code to modify a static field must synchronize access to that field. That is necessary because there is no guarantee that untrusted clients will externally synchronize when accessing the field. Because a static field is shared by all clients, untrusted clients may violate the contract by failing to provide suitable locking.
According to Bloch [Bloch 2008]
If a method modifies a static field, you must synchronize access to this field, even if the method is typically used only by a single thread. It is not possible for clients to perform external synchronization on such a method because there can be no guarantee that unrelated clients will do likewise.
Documented design intent is irrelevant when dealing with untrusted code because an attacker can always choose to ignore the documentation.
3.6.1Noncompliant Code Example
This noncompliant code example does not synchronize access to the static counter field.
/** This class is not thread-safe */ public final class CountHits {
private static int counter;
public void incrementCounter() { counter++;
}
}
This class definition does not violate guideline “VNA02-J. Ensure that compound operations on shared variables are atomic” on page 16, which only applies to classes that promise thread-safety. However, this class has a mutable static counter field that is modified by the publicly accessible incrementCounter() method. Consequently, this class cannot be used securely by trusted client code, if untrusted code can purposely fail to externally synchronize access to the field.
CMU/SEI-2010-TR-015 | 59
LCK05-J
3.6.2Compliant Solution
This compliant solution uses a static private final lock to protect the counter field and, consequently, does not depend on any external synchronization. This solution also complies with guideline “LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code” on page 41.
/** This class is thread-safe */ public final class CountHits { private static int counter;
private static final Object lock = new Object();
public void incrementCounter() { synchronized (lock) {
counter++;
}
}
}
3.6.3Risk Assessment
Failing to internally synchronize access to static fields that may be modified by untrusted code will result in incorrectly synchronized code, if the author of the untrusted code chooses to ignore the synchronization policy.
Guideline |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
LCK05- J |
low |
probable |
medium |
P4 |
L3 |
3.6.4References
[Bloch 2008] |
Item 67: “Avoid excessive synchronization” |
[Sun 2009b]
CMU/SEI-2010-TR-015 | 60
LCK06-J
3.7LCK06-J. Do not use an instance lock to protect shared static data
Shared static data should not be protected using instance locks because they are ineffective when two or more instances of the class are created. Consequently, shared state is not safe for concurrent access unless a static lock object is used. If the class can interact with untrusted code, the lock must also be private and final, as per guideline “LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code” on page 41.
3.7.1Noncompliant Code Example (Non-Static Lock Object for Static Data)
This noncompliant code example uses a non-static lock object to guard access to a static counter field. If two Runnable tasks are started, they will create two instances of the lock object and lock on each one separately.
public final class CountBoxes implements Runnable { private static volatile int counter;
// ...
private final Object lock = new Object();
@Override public void run() { synchronized(lock) {
counter++; // ...
}
}
public static void main(String[] args) { for(int i = 0; i < 2; i++) {
new Thread(new CountBoxes()).start();
}
}
}
This example does not prevent either thread from observing an inconsistent value of counter because the increment operation on volatile fields is non-atomic in the absence of proper synchronization (see guideline “VNA02-J. Ensure that compound operations on shared variables are atomic” on page 16).
CMU/SEI-2010-TR-015 | 61