You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 46 Next »

Static shared data should not be protected using instance locks because these are ineffective when two or more instances of the class are created. Consequently, the 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 in rule LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code.

Noncompliant 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 not atomic in the absence of proper synchronization. (See rule VNA02-J. Ensure that compound operations on shared variables are atomic.)

Noncompliant Code Example (Method Synchronization for Static Data)

This noncompliant code example uses method synchronization to protect access to a static class counter field.

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

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

In this case, the intrinsic lock is associated with each instance of the class and not 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 declares the lock object as static and, consequently, ensures the atomicity of the increment operation.

public class CountBoxes implements Runnable {
  private static int counter;
  // ...
  private static final Object lock = new Object();

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

There is no need 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 static data

No

No obvious issues

method synchronization for static data

No

No obvious issues

The following table summarizes the examples flagged as violations by SureLogic JSure:

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

modeling problem: static region "counter" should be protected by a static field

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Bibliography

[[API 2006]]

Issue Tracking

100%

Review List

  1. handler

    "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.

    Priority MEDIUM
    rcs_mgr
    Apr 02, 2010

      08. Locking (LCK)      

  • No labels