Programs must not use instance locks to protect static shared data because instance locks are ineffective when two or more instances of the class are created. Consequently, failure to use a static lock object leaves the shared state unprotected against concurrent access. Lock objects for classes that can interact with untrusted code must also be private and final, as shown in rule LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code.
Noncompliant Code Example (
...
Nonstatic Lock Object for Static Data)
This noncompliant code example attempts to guard access to a the static counter
field using a non-static nonstatic lock object. When two Runnable
tasks are started, they will create two instances of the lock object and will lock on each instance separately.
Code Block | ||
---|---|---|
| ||
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 fails to prevent either thread from observing an inconsistent value of counter
because the increment operation on volatile fields fails to be atomic in the absence of proper synchronization. (See rule see VNA02-J. Ensure that compound operations on shared variables are atomic for more information).)
Noncompliant Code Example (Method Synchronization for Static Data)
This noncompliant code example uses method synchronization to protect access to a static class counter
field.:
Code Block | ||
---|---|---|
| ||
public final class CountBoxes implements Runnable {
private static volatile int counter;
// ...
public synchronized void run() {
counter++;
// ...
}
// ...
}
|
In this case, the method synchronization uses the intrinsic lock associated with each instance of the class rather than the intrinsic lock associated with the class itself. Consequently, threads constructed using different Runnable
instances may observe inconsistent values of the counter
.
Compliant Solution (Static Lock Object)
This compliant solution ensures the atomicity of the increment operation by declaring the lock object as static.locking on a static object:
Code Block | ||
---|---|---|
| ||
public class CountBoxes implements Runnable {
private static int counter;
// ...
private static final Object lock = new Object();
public void run() {
synchronized (lock) {
counter++;
// ...
}
}
// ...
}
|
It is unnecessary to declare the counter
variable volatile when using synchronization.
Risk Assessment
Using an instance lock to protect static shared data can result in nondeterministic behavior.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
LCK06-J |
Medium |
Probable |
Medium | P8 | L2 |
Automated Detection
The following table summarizes the examples flagged as violations by SureLogic Flashlight:
Noncompliant Code Example | Flagged | Message |
---|---|---|
non-static lock object for | No | No obvious issues |
method synchronization for | No | No obvious issues |
The following table summarizes the examples flagged as violations by SureLogic JSure:
Some static analysis tools can detect violations of this rule.
Tool | Version | Checker | Description | ||||||
---|---|---|---|---|---|---|---|---|---|
Parasoft Jtest |
| CERT.LCK06.INSTLOCK | Do not use an instance lock to protect shared static data | ||||||
SpotBugs |
| SSD_DO_NOT_USE_INSTANCE_LOCK_ON_SHARED_STATIC_DATA | Implemented since 4.6.0 |
Noncompliant Code Example
Flagged
Message
non-static lock object for static
data
Yes
modeling problem: static region "counter" should be protected by a static field
method synchronization for static
data
Yes
Related Guidelines
Bibliography
...
Issue Tracking
Tasklist | ||||
---|---|---|---|---|
| ||||
||Completed||Priority||Locked||CreatedDate||CompletedDate||Assignee||Name|| |T|M|F|1270215165305|1271447005294|rcs_mgr|"Ideally, the lock should also be private and final" => I have been penalized for using "ideally" before. I would just remove that sentence or make it more definitive. Note that we also have an NCE that uses method synchronization. Perhaps we also need a CS with a static method.| |
...