Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

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.

...

This noncompliant code example attempts to guard access to the static counter field using a non-static nonstatic lock object. When two Runnable tasks are started, they create two instances of the lock object and lock on each instance separately.

Code Block
bgColor#FFcccc

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
bgColor#FFcccc

public final class CountBoxes implements Runnable {
  private static volatile int counter;
  // ...

  public synchronized void run() {
    counter++;
    // ...
  }
  // ...
}

...

This compliant solution ensures the atomicity of the increment operation by locking on a static object.:

Code Block
bgColor#ccccff

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

Medium

probable

Probable

medium

Medium

P8

L2

Automated Detection

Some static analysis tools can detect violations of this rule.

ToolVersionCheckerDescription
Parasoft Jtest

Include Page
Parasoft_V
Parasoft_V

CERT.LCK06.INSTLOCKDo not use an instance lock to protect shared static data
SpotBugs

Include Page
SpotBugs_V
SpotBugs_V

SSD_DO_NOT_USE_INSTANCE_LOCK_ON_SHARED_STATIC_DATAImplemented since 4.6.0

Related Guidelines

MITRE CWE

CWE-667

.

, Improper Locking

Bibliography

<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="ec26a958-5e73-4356-bf6a-e28f32f45efc"><ac:plain-text-body><![CDATA[

[[API 2006

AA. Bibliography#API 06]]

]]></ac:plain-text-body></ac:structured-macro>

[API 2014]

Issue Tracking

Tasklist
Review List
Review List
 
||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.| 


...

Image Removed      08. Locking (LCK)      Image Added Image Added