Page History

public final

Static shared data should not be protected using instance locks because the instance locks are ineffective when two or more instances of the class are created. Consequently, the shared state is not safe for concurrent access.

h2. 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}} field {{counter}}. If two {{Runnable}} tasks, each consisting of a thread are started, they will create two instances of the lock object and lock on each separately. 

{mc} Do not declare the classes non-public and final, otherwise threads will be unable access counter {mc}

{code:bgColor=#FFcccc}
public 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 Runnable(int r1i = 0; i new< CountBoxes()2;
 i++) {
  Thread t1 = new Thread(r1);
    Runnable r2 = new CountBoxes();
    Thread t2 = new Thread(r2);
    t1.start();
    t2.start();}
  }
}
{code}

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


h2. Noncompliant Code Example (method synchronization for {{static}} data)

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

{code:bgColor=#FFcccc}
public

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

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

The problem is that this lock is associated with each instance of the class and not with the class object itself. Consequently, threads constructed using different {{Runnable}} instances may observe inconsistent values of the {{counter}}.


h2. Compliant Solution ({{static}} lock object)

This compliant solution declares the lock object as {{static}} and consequently, ensures the atomicity of the increment operation.

{code

:bgColor=#ccccff}
public class CountBoxes implements Runnable {
  private static int counter;
  // ...

  private static final Object lock = new Object();    
  
  public void run() {
    synchronized (lock) {
      counter++; 
      // ...
    }
  }
  // ...
}
{code}

There is no requirement of declaring the {{counter}} variable as {{volatile}} when synchronization is used.

h2. Risk Assessment

Using an instance lock to protect static shared data provides no synchronization properties and can lead to non-deterministic behavior.

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON34- J | medium | probable | medium | {color:#cc9900}{*}P8{*}{color} | {color:#cc9900}{*}L2{*}{color} |



h3. Automated Detection

TODO

h3. Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the [CERT website|https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON34-J].

h2. References

\[[API 06|AA. Java References#API 06]\] 


----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|VOID CON00-J. Synchronize access to shared mutable variables]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON03-J. Do not use background threads during class initialization]

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