SEI CERT Oracle Coding Standard for Java

Space shortcuts

Page tree

Versions Compared

Old Version 78

changes.mady.by.user Tim Halloran

Saved on

compared with

New Version 79

changes.mady.by.user Dhruv Mohindra

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: made some changes to flag toggle examples, please review ... removed old hidden text

Wiki Markup
Compound operations on shared variables (consisting of more than one discrete operation) must be performed atomically. Errors can arise from compound operations that need to be perceived atomically but are not \[[JLS 05|AA. Java References#JLS 05]\].

...

Compound 
...
 assignment 
...
 expressions 
...
 include 
...
 operators 
...
*=, 
...
 /=, 
...
 %=, 
...
 +=, 
...
 -=, 
...
 <<=, 
...
 >>=, 
...
 >>>=, 
...
 ^=, 
...
 or 
...
 |=
...
. 
...
 The 
...
 postfix 
...
 and 
...
 prefix 
...
 increment 
...
 (
...
+
...
+
...
) 
...
 and 
...
 decrement 
...
 (
...
-
...
-
...
) 
...
 operations 
...
 can 
...
 also 
...
 be 
...
 treated 
...
 as 
...
 compound 
...
 expressions. 
...
For 
...
 atomicity 
...
 of 
...
 a 
...
 grouping 
...
 of 
...
 calls 
...
 to 
...
 independently 
...
 atomic 
...
 methods 
...
 of 
...
 the 
...
 existing 
...
 Java 
...
 API, 
...
 see 
...
CON07-J. 
...
 Do 
...
 not 
...
 assume 
...
 that 
...
 a 
...
 grouping 
...
 of 
...
 calls 
...
 to 
...
 independently 
...
 atomic 
...
 methods 
...
 is 
...
 atomic
...
.
...
The 
...
 Java 
...
 Language 
...
 Specification 
...
 also 
...
 permits 
...
 reads 
...
 and 
...
 writes 
...
 of 
...
 64-bit 
...
 values 
...
 to 
...
 be 
...
 non-atomic 
...
 though 
...
 this 
...
 is 
...
 not 
...
 an 
...
 issue 
...
 with 
...
 most 
...
 modern 
...
 JVMs 
...
 (see 
...
CON25-J. 
...
 Ensure 
...
 atomicity 
...
 when 
...
 reading 
...
 and 
...
 writing 
...
 64-bit 
...
 values
...
).
...
Noncompliant 
...
 Code 
...
 Example 
...
 (bitwise 
...
 compound 
...
 operation)
...
This 
...
 noncompliant 
...
 code 
...
 example 
...
 declares 
...
 a 
...
 shared 
...
boolean
...
 variable 
...
flag
...
 and 
...
 uses 
...
 an 
...
 optimization 
...
 in 
...
 the 
...
toggle()
...
 method 
...
 to 
...
 negate 
...
 the 
...
 current 
...
 value 
...
 of 
...
 the 
...
 flag.


{:=
 Code Block
bgColor
#FFcccc
}
class Foo {
  private boolean flag = true;
 
  public void toggle() {  // unsafe
    flag ^= true; // same as flag = !flag; 
  }
}
{code}

However, this code is not 
  public Flag getFlag() { // unsafe 
    return flag;
  }
}

However, this code is not thread-safe. 
...
 Multiple 
...
 threads 
...
 may 
...
 not 
...
 observe 
...
 the 
...
 latest 
...
 state 
...
 of 
...
 the 
...
flag
...
 because 
...
^=
...
 constitutes a 
...
 non-atomic 
...
 operation.
...
Noncompliant 
...
 Code 
...
 Example 
...
 (
...
volatile
...
 variable)
...
This 
...
 noncompliant 
...
 code 
...
 example 
...
 derives 
...
 from 
...
 the 
...
 preceding 
...
 one 
...
 but 
...
 declares 
...
 the 
...
flag
...
 as 
...
 volatile. 
 Code Block
bgColor#FFcccc
}}. 

{code:bgColor=#FFcccc}
class Foo {
  private volatile boolean flag = true;
 
  public booleanvoid toggle() {  // unsafe
    flag ^= true; //
 same as}

 flag =public !flag; 
    Flag getFlag() { // safe
    return flag;
  }
}
{code}


It 
...
 is 
...
 still 
...
 insecure 
...
 for 
...
 multithreaded 
...
 use 
...
 because 
...
volatile
...
 does 
...
 not 
...
 guarantee 
...
 the 
...
 visibility 
...
 of 
...
 updates 
...
 to 
...
 the 
...
 shared 
...
 variable 
...
flag
...
 when 
...
 a 
...
 compound 
...
 operation 
...
 is 
...
 performed.
...
Compliant 
...
 Solution 
...
 (synchronization)
...
This 
...
 compliant 
...
 solution 
...
 synchronized 
...
 the 
...
toggle()
...
 method 
...
 to 
...
 ensure 
...
 that 
...
 the 
...
flag
...
 is 
...
 made 
...
 visible 
...
 to 
...
 all 
...
 the 
...
 threads.


{:=
 Code Block
bgColor
#ccccff
}
class Foo {
  private volatile boolean flag = true;
 
  public synchronized booleanvoid toggle() { 
    flag ^= true; // same as flag = !flag; 
  }

  public Flag getFlag() { 
    return flag;
  }
}
{code}

h2. Compliant Solution ({{
Compliant Solution (java.util.concurrent.atomic.AtomicBoolean
...
)
...
This 
...
 compliant 
...
 solution 
...
 uses 
...
 the 
...
java.util.concurrent.atomic.AtomicBoolean
...
 type 
...
 to 
...
 declare 
...
 the 
...
 flag. 
 Code Block
bgColor#ccccff
}}. 

{code:bgColor=#ccccff}
class Foo {
  private AtomicBoolean flag = new AtomicBoolean(true);
 
  public booleanvoid toggle() { 
    boolean temp;
    do {
      temp = flag.get();
    } while(!flag.compareAndSet(temp, !temp));
  }

  public Flag getFlag() { 
    return flag.get();
  }
}
{code}


It 
...
 ensures 
...
 that 
...
 updates 
...
 to 
...
 the 
...
 variable 
...
 are 
...
 carried 
...
 out 
...
 by 
...
 using 
...
 the 
...
compareAndSet()
...
 method 
...
 of 
...
 the 
...
 class 
...
AtomicBoolean
...
. 
...
 All 
...
 updates 
...
 are 
...
 made 
...
 visible 
...
 to 
...
 other 
...
 threads.
Noncompliant Code Example (increment/decrement)
Prefix and postfix, increment and decrement operations are non-atomic in that the value written depends upon the value initially read from the operand. For example, x++ is non-atomic because it is a composite operation consisting of three discrete operations: reading the current value of x, adding one to it, and writing the new, incremented value back to x. 
This noncompliant code example contains a data race that may result in the itemsInInventory field failing to account for removed items.
 Code Block
bgColor#FFcccc


{mc}
// THIS CONTENT IS CURRENTLY HIDDEN
h2. Noncompliant Code Example (bitwise logic)

This class maintains a set of flags, which can be set and cleared independently. They are stored in a single byte field.

{code:bgColor=#FFcccc}
class Flags {
  public static final byte FLAG_1 = 1
  public static final byte FLAG_2 = 2;
  public static final byte FLAG_4 = 4;
  public static final byte FLAG_8 = 8;

  private byte flags = 0;

  public void setFlag(byte flag) {
    flags |= flag;
  }

  public void clearFlag(byte flag) {
    flags &= ~flag;
  }
} 
{code}

This class is not thread-safe at all, even though only one value is modified.  For instance, suppose Thread 1 calls {{setFlag( FLAG_1)}} while Thread 2 calls {{setFlag( FLAG_2)}}. The following represents one possible execution of the two threads:

||Time||flags=||Thread||Action||
|1|0|_t_~1~|reads the current value of {{flags}}, 0, into a temporary variable|
|2|0|_t_~2~|reads the current value of {{flags}}, (still) 0, into a temporary variable |
|3|0|_t_~1~|sets the 1st byte, creating the value 1|
|4|0|_t_~2~|sets the 2nd byte, creating the value 2|
|5|1| _t_~1~|writes the temporary variable value to {{flags}}|
|6|2| _t_~2~|writes the temporary variable value to {{flags}}|

As a result, the effect of the call by _t_~1~ is not reflected in {{flags}}; the program behaves as if the call was never made. 

Furthermore, it is quite likely that if one thread sets a flag, and another thread retrieves the flags value, the second thread will not see the setting from the first thread.


h2. Noncompliant Code Example (volatile)

In this noncompliant code example, the {[flags}} field is {{volatile}}.

{code:bgColor=#FFcccc}
class Flags {
  public static final byte FLAG_1 = 1
  public static final byte FLAG_2 = 2;
  public static final byte FLAG_4 = 4;
  public static final byte FLAG_8 = 8;

  private volatile byte flags = 0;

  public void setFlag(byte flag) {
    flags |= flag;
  }

  public void clearFlag(byte flag) {
    flags &= ~flag;
  }
} 
{code}

The {{volatile}} keyword guarantees that any writes to the {{flags}} variable will be seen by any subsequent reads. However, the {{volatile}} keyword does not prevent the exedcution scenario described above. In particular, it does not guarantee that the read of the {{flags}} variable followed by the write of the {{flags}} variable is atomic.


h2. Compliant Solution ({{java.util.concurrent.atomic}} classes)

The {{java.util.concurrent}} utilities can be used to atomically manipulate a shared variable. In this compliant solution, {{flags}} is an {{AtomicInteger}}, allowing composite operations to be performed atomically.

{code:bgColor=#ccccff}
class Flags {
  private AtomicInteger flags = new AtomicInteger( 0);

  public void setFlag(byte flag) {
    while (true) {
      int old = flags.get();
      int next = old | flag;
      if (flags.compareAndSet( old, next)) {
        break;
      }
    }
  }

  public void clearFlag(byte flag) {
    while (true) {
      int old = flags.get();
      int next = old & ~flag;
      if (flags.compareAndSet( old, next)) {
        break;
      }
    }
  }
} 

{code}

Note that updates to shared atomic variables are visible to other threads.

The {{compareAndSet()}} method takes two arguments, the expected value of a variable when the method is invoked and the updated value. This compliant solution uses this method to atomically set the value of {{flags}} to the updated value if and only if the current value equals the expected value \[[API 06|AA. Java References#API 06]\].  The {{while}} loop ensures that each  method persists in trying to set {{flags}} until it succeeds.


h2. Compliant Solution (synchronization)

This compliant solution uses synchronization to protect access to the {{flags}} field. Synchronization provides a way to safely share object state across multiple threads without the need to reason about reorderings, compiler optimizations, and hardware specific behavior.

{code:bgColor=#ccccff}
class Flags {
  private byte flags = 0;
  private Object lock = new Object();

  public void setFlag(byte flag) {
    synchronized (lock) {
      flags |= flag;
    }
  }

  public void clearFlag(byte flag) {
    synchronized (lock) {
      flags &= ~flag;
    }
  }
} 
{code}

If code is synchronized correctly, updates to shared variables are instantly made visible to other threads. Synchronization is more expensive than using the optimized {{java.util.concurrent}} utilities and should generally be preferred when it is sufficiently complex to carry out the operation atomically using the utilities. When using synchronization, care must be taken to avoid deadlocks (see [CON12-J. Avoid deadlock by requesting and releasing locks in the same order]).

Constructors and methods can use block synchronization as an alternative to method synchronization. Block synchronization synchronizes a block of code rather than a method, as shown in this compliant solution. Block synchronization can also synchronize on a lock besides the object's intrinsic lock, as is recommended by [CON04-J. Use the private lock object idiom instead of the Class object's intrinsic locking mechanism].

// END OF HIDDEN CONTENT
{mc}

h2. Noncompliant Code Example (increment/decrement)

Prefix and postfix, increment and decrement operations are non-atomic in that the value written depends upon the value initially read from the operand.  For example, {{x+\+}} is non-atomic because it is a composite operation consisting of three discrete operations: reading the current value of {{x}}, adding one to it, and writing the new, incremented value back to {{x}}.  

This noncompliant code example contains a data race that may result in the {{itemsInInventory}} field failing to account for removed items.

{code:bgColor=#FFcccc}
class InventoryManager {
  private static final int MIN_INVENTORY = 3;
  private int itemsInInventory = 100;

  public final void removeItem() {
    if (itemsInInventory <= MIN_INVENTORY) {
      throw new IllegalStateException("Under stocked");
    }
    itemsInInventory--;
  }
} 
{code}


For 
...
 example, 
...
 if 
...
 the 
...
removeItem()
...
 method 
...
 is 
...
 concurrently 
...
 invoked 
...
 by 
...
 two 
...
 threads, 
...
t
...
1 and 
...
t
...
2, 
...
 the 
...
 execution 
...
 of 
...
 these 
...
 threads 
...
 may 
...
 be 
...
 interleaved 
...
 so 
...
 that:
...
Time
itemsInInventory=
...
Thread
Action
1
100
t1
reads the current value of itemsInInventory, 100, into a temporary variable
2
100
t2
reads the current value of itemsInInventory, (still) 100, into a temporary variable 
3
100
t1
decrements the temporary variable to 99
4
100
t2
decrements the temporary variable to 99
5
99
 t1
writes the temporary variable value to itemsInInventory
6
99
 t2
writes the temporary variable value to itemsInInventory
As a result, the effect of the call by t1 is not reflected in itemsInInventory; the program behaves as if the call was never made. 
As another example, suppose itemsInInventory currently has the value MIN_INVENTORY + 1. If the removeItem() method is concurrently invoked by two threads, t1 and t2, the execution of these threads may be interleaved so that:
Time
itemsInInventory=
Thread
Action
1
MIN_INVENTORY+1
t1
checks that the current value of itemsInInventory is large enough to decrement, which it is
2
MIN_INVENTORY+1
t2
checks that the current value of itemsInInventory is large enough to decrement, which it is
3
MIN_INVENTORY+1
t1
reads the current value of itemsInInventory, MIN_INVENTORY+1, into a temporary variable 
4
MIN_INVENTORY
t1
decrements the temporary variable to MIN_INVENTORY
5
MIN_INVENTORY
 t1
writes the temporary variable value to itemsInInventory
6
MIN_INVENTORY
t2
reads the current value of itemsInInventory, MIN_INVENTORY, into a temporary variable 
7
MIN_INVENTORY-1
t2
decrements the temporary variable to MIN_INVENTORY-1
8
MIN_INVENTORY-1
 t2
writes the temporary variable value to itemsInInventory
As a result, both threads decrement itemsInInventory but the range check on the variable is bypassed, causing the variable to have an invalid value. The decrement operation may even wrap if MIN_INVENTORY == Integer.MIN_VALUE.
Noncompliant Code Example (volatile)
This noncompliant code example attempts to resolve the problem by declaring itemsInInventory volatile.
 Code Block
bgColor#FFcccc
||Action||
|1|100|_t_~1~|reads the current value of {{itemsInInventory}}, 100, into a temporary variable|
|2|100|_t_~2~|reads the current value of {{itemsInInventory}}, (still) 100, into a temporary variable |
|3|100|_t_~1~|decrements the temporary variable to 99|
|4|100|_t_~2~|decrements the temporary variable to 99|
|5|99| _t_~1~|writes the temporary variable value to {{itemsInInventory}}|
|6|99| _t_~2~|writes the temporary variable value to {{itemsInInventory}}|

As a result, the effect of the call by _t_~1~ is not reflected in {{itemsInInventory}}; the program behaves as if the call was never made. 


As another example, suppose itemsInInventory currently has the value MIN_INVENTORY + 1.  If the {{removeItem()}} method is concurrently invoked by two threads, _t_~1~ and _t_~2~, the execution of these threads may be interleaved so that:

||Time||itemsInInventory=||Thread||Action||
|1|MIN_INVENTORY+1|_t_~1~|checks that the current value of {{itemsInInventory}} is large enough to decrement, which it is|
|2|MIN_INVENTORY+1|_t_~2~|checks that the current value of {{itemsInInventory}} is large enough to decrement, which it is|
|3|MIN_INVENTORY+1|_t_~1~|reads the current value of {{itemsInInventory}}, MIN_INVENTORY+1, into a temporary variable |
|4|MIN_INVENTORY|_t_~1~|decrements the temporary variable to MIN_INVENTORY|
|5|MIN_INVENTORY| _t_~1~|writes the temporary variable value to {{itemsInInventory}}|
|6|MIN_INVENTORY|_t_~2~|reads the current value of {{itemsInInventory}}, MIN_INVENTORY, into a temporary variable |
|7|MIN_INVENTORY-1|_t_~2~|decrements the temporary variable to MIN_INVENTORY-1|
|8|MIN_INVENTORY-1| _t_~2~|writes the temporary variable value to {{itemsInInventory}}|

As a result, both threads decrement {{itemsInInventory}} but the range check on the variable is bypassed, causing the variable to have an invalid value. The decrement operation may even wrap if {{MIN_INVENTORY == Integer.MIN_VALUE}}.


h2. Noncompliant Code Example (volatile)

This noncompliant code example attempts to resolve the problem by declaring {{itemsInInventory}} volatile.

{code:bgColor=#FFcccc}
class InventoryManager {
  private static final int MIN_INVENTORY = 3;
  private volatile int itemsInInventory = 100;

  public final void removeItem() {
    if (itemsInInventory <= MIN_INVENTORY) {
      throw new IllegalStateException("under stocked");
    }
    itemsInInventory--;
  }
} 
{code}


Volatile 
...
 variables 
...
 are 
...
 unsuitable 
...
 when 
...
 more 
...
 than 
...
 one 
...
 read/write 
...
 operation 
...
 needs 
...
 to 
...
 be 
...
 atomic. 
...
 The 
...
 use 
...
 of 
...
 a 
...
 volatile 
...
 variable 
...
 in 
...
 this 
...
 noncompliant 
...
 code 
...
 example 
...
 guarantees 
...
 that 
...
 once 
...
itemsInInventory
...
 has 
...
 been 
...
 updated, 
...
 the 
...
 new 
...
 value 
...
 is 
...
 visible 
...
 to 
...
 all 
...
 threads 
...
 that 
...
 read 
...
 the 
...
 field. 
...
 However, 
...
 because 
...
 the 
...
 post 
...
 decrement 
...
 operator 
...
 is 
...
 nonatomic, 
...
 even 
...
 when 
...
volatile
...
 is 
...
 used, 
...
 the 
...
 interleaving 
...
 described 
...
 in 
...
 the 
...
 previous 
...
 noncompliant 
...
 code 
...
 example 
...
 is 
...
 still 
...
 possible. 
...
 Furthermore, 
...
 the 
...
 race 
...
 codnition 
...
 imposed 
...
 by 
...
 range-checking 
...
itemsInInventory
...
 before 
...
 decrementing 
...
 it 
...
 is 
...
 also 
...
 still 
...
 possible.
...
Compliant Solution (java.util.concurrent.atomic
...
 classes)
...
The 
...
java.util.concurrent
...
 utilities 
...
 can 
...
 be 
...
 used 
...
 to 
...
 atomically 
...
 manipulate 
...
 a 
...
 shared 
...
 variable. 
...
 This 
...
 compliant 
...
 solution 
...
 defines 
...
intemsInInventory
...
 as 
...
 a 
...
java.util.concurrent.atomic.AtomicInteger
...
 variable, 
...
 allowing 
...
 composite 
...
 operations 
...
 to 
...
 be 
...
 performed 
...
 atomically.


{:=
 Code Block
bgColor
#ccccff
}
class InventoryManager {
  private static final int MIN_INVENTORY = 3;
  private final AtomicInteger itemsInInventory = new AtomicInteger(100);

  public final void removeItem() {
    while (true) {
      int old = itemsInInventory.get();
      if (old <= MIN_INVENTORY) {
        throw new IllegalStateException("Under stocked");
      }
      int next = old - 1; // Decrement
      if (itemsInInventory.compareAndSet(old, next)) {
        break;
      }
    } // end while
  } // end removeItem()
} 
{code}


Note 
...
 that 
...
 updates 
...
 to 
...
 shared 
...
 atomic 
...
 variables 
...
 are 
...
 visible 
...
 to 
...
 other 
...
 threads.
...
 Wiki Markup
The {{compareAndSet()}} method takes two arguments, the expected value of a variable when the method is invoked and the updated value. This compliant solution uses this method to atomically set the value of {{itemsInInventory}} to the updated value if and only if the current value equals the expected value \[[API 06|AA. Java References#API 06]\].  The {{while}} loop ensures that the {{removeItem()}} method succeeds in decrementing the most recent value of {{itemsInInventory}} as long as the inventory count is greater than {{MIN_INVENTORY}}.
...
Compliant Solution (method 
...
 synchronization)
...
Synchronization 
...
 provides 
...
 a 
...
 way 
...
 to 
...
 safely 
...
 share 
...
 object 
...
 state 
...
 across 
...
 multiple 
...
 threads 
...
 without 
...
 the 
...
 need 
...
 to 
...
 reason 
...
 about 
...
 reorderings, 
...
 compiler 
...
 optimizations, 
...
 and 
...
 hardware 
...
 specific 
...
 behavior.
...
This 
...
 compliant 
...
 solution 
...
 uses 
...
 method 
...
 synchronization 
...
 to 
...
 synchronize 
...
 access 
...
 to 
...
itemsInInventory
...
. 
...
 Consequently, 
...
 access 
...
 to 
...
itemsInInventory
...
 is 
...
 mutually 
...
 exclusive 
...
 and 
...
 its 
...
 state 
...
 consistent 
...
 across 
...
 all 
...
 threads.


{:=
 Code Block
bgColor
#ccccff
}
class InventoryManager {
  private static final int MIN_INVENTORY = 3;
  private int itemsInInventory = 100;

  public final synchronized void removeItem() {
    if (itemsInInventory <= MIN_INVENTORY) {
      throw new IllegalStateException("Under stocked");
    }
    itemsInInventory--;
  }
} 
{code}


If 
...
 code 
...
 is 
...
 synchronized 
...
 correctly, 
...
 updates 
...
 to 
...
 shared 
...
 variables 
...
 are 
...
 instantly 
...
 made 
...
 visible 
...
 to 
...
 other 
...
 threads. 
...
 Synchronization 
...
 is 
...
 more 
...
 expensive 
...
 than 
...
 using 
...
 the 
...
 optimized 
...
java.util.concurrent
...
 utilities 
...
 and 
...
 should 
...
 generally 
...
 be 
...
 preferred 
...
 when 
...
 it 
...
 is 
...
 sufficiently 
...
 complex 
...
 to 
...
 carry 
...
 out 
...
 the 
...
 operation 
...
 atomically 
...
 using 
...
 the 
...
 utilities. 
...
 When 
...
 using 
...
 synchronization, 
...
 care 
...
 must 
...
 be 
...
 taken 
...
 to 
...
 avoid 
...
 deadlocks 
...
 (see 
...
CON12-J. 
...
 Avoid 
...
 deadlock 
...
 by 
...
 requesting 
...
 and 
...
 releasing 
...
 locks 
...
 in 
...
 the 
...
 same 
...
 order
...
).
...
Compliant 
...
 Solution 
...
 (block 
...
 synchronization)
...
Constructors 
...
 and 
...
 methods 
...
 can 
...
 use 
...
 block 
...
 synchronization 
...
 as 
...
 an 
...
 alternative 
...
 to 
...
 method 
...
 synchronization. 
...
 Block 
...
 synchronization 
...
 synchronizes 
...
 a 
...
 block 
...
 of 
...
 code 
...
 rather 
...
 than 
...
 a 
...
 method, 
...
 as 
...
 shown 
...
 in 
...
 this 
...
 compliant 
...
 solution.


{:=
 Code Block
bgColor
#ccccff
}
class InventoryManager {
  private static final int MIN_INVENTORY = 3;
  private int itemsInInventory = 100;
  private final Object lock = new Object();

  public final void removeItem() {
    synchronized(lock) {
      if (itemsInInventory <= MIN_INVENTORY) {
        throw new IllegalStateException("Under stocked");
      }
      itemsInInventory--;
    }
  }
} 
{code}


Block 
...
 synchronization 
...
 is 
...
 preferable 
...
 over 
...
 method 
...
 synchronization 
...
 because 
...
 it 
...
 enables 
...
 reduction 
...
 of 
...
 the 
...
 duration 
...
 for 
...
 which 
...
 the 
...
 lock 
...
 is 
...
 held. 
...
 This 
...
 is 
...
 because 
...
 statements 
...
 that 
...
 do 
...
 not 
...
 require 
...
 synchronization 
...
 can 
...
 be 
...
 safely 
...
 moved 
...
 out 
...
 of 
...
 the 
...
 synchronized 
...
 block. 
...
 This 
...
 compliant 
...
 solution 
...
 requires 
...
 all 
...
 statements 
...
 to 
...
 be 
...
 synchronized 
...
 and 
...
 consequently, 
...
 is 
...
 comparable 
...
 to 
...
 the 
...
 previous 
...
 compliant 
...
 solution 
...
 with 
...
 respect 
...
 to 
...
 performance.
...
Block 
...
 synchronization 
...
 when 
...
 used 
...
 in 
...
 conjunction 
...
 with 
...
 a 
...
private
...
 internal 
...
 lock 
...
 object 
...
 also 
...
 protects 
...
 against 
...
 denial 
...
 of 
...
 service 
...
 attacks. 
...
 Block 
...
 synchronization 
...
 does 
...
 not 
...
 require 
...
 synchronizing 
...
 on 
...
 an 
...
 internal 
...
 private 
...
 lock 
...
 object 
...
 instead 
...
 of 
...
 the 
...
 intrinsic 
...
 lock 
...
 of 
...
 the 
...
 class's 
...
 object 
...
 (
...
this
...
 reference). 
...
 However, 
...
 it 
...
 is 
...
 more 
...
 secure 
...
 to 
...
 synchronize 
...
 on 
...
 an 
...
 internal 
...
 private 
...
 lock 
...
 object 
...
 instead 
...
 of 
...
 a 
...
 more 
...
 accessible 
...
 lock 
...
 object. 
...
 See 
...
CON04-J. 
...
 Use 
...
 the 
...
 private 
...
 lock 
...
 object 
...
 idiom 
...
 instead 
...
 of 
...
 the 
...
 Class 
...
 object's 
...
 intrinsic 
...
 locking 
...
 mechanism
...
 for 
...
 more 
...
 information.
...
Compliant Solution (ReentrantLock)
This compliant solution uses a java.util.concurrent.locks.ReentrantLock
...
 to 
...
 atomically 
...
 perform 
...
 the 
...
 post-decrement 
...
 operation.


{:=
 Code Block
bgColor
#ccccff
}
class InventoryManager {
  private static final int MIN_INVENTORY = 3;
  private int itemsInInventory = 100;
  private final Lock lock = new ReentrantLock();

  public final void removeItem() {
    if (lock.tryLock()) {
      try {
        if (itemsInInventory <= MIN_INVENTORY) {
          throw new IllegalStateException("Under stocked");
        }
        itemsInInventory--;
      } finally {
        lock.unlock();
      }
    }
  } // end removeItem()
} 
{code}


Code 
...
 that 
...
 uses 
...
 this 
...
 lock 
...
 behaves 
...
 similar 
...
 to 
...
 synchronized 
...
 code 
...
 that 
...
 uses 
...
 the 
...
 traditional 
...
 monitor 
...
 lock. 
...
ReentrantLock
...
 provides 
...
 several 
...
 other 
...
 capabilities, 
...
 for 
...
 instance, 
...
 the 
...
tryLock()
...
 method 
...
 does 
...
 not 
...
 block 
...
 waiting 
...
 if 
...
 another 
...
 thread 
...
 is 
...
 already 
...
 holding 
...
 the 
...
 lock. 
...
 The 
...
 class 
...
java.util.concurrent.locks.ReentrantReadWriteLock
...
 can 
...
 be 
...
 used 
...
 when 
...
 some 
...
 thread 
...
 requires 
...
 a 
...
 lock 
...
 to 
...
 write 
...
 information 
...
 while 
...
 other 
...
 threads 
...
 require 
...
 the 
...
 lock 
...
 to 
...
 concurrently 
...
 read 
...
 the 
...
 information.
...
Noncompliant Code Example (addition, 
...
 volatile 
...
 fields)
...
In 
...
 this 
...
 noncompliant 
...
 code 
...
 example, 
...
 the 
...
 two 
...
 fields 
...
a
...
 and 
...
b
...
 may 
...
 be 
...
 set 
...
 by 
...
 multiple 
...
 threads, 
...
 using 
...
 the 
...
setValues()
...
 method.


{:=
 Code Block
bgColor
#FFcccc
}
private volatile int a;
private volatile int b;

public int getSum() throws ArithmeticException {
  // Check for integer overflow
  if( b > 0 ? a > Integer.MAX_VALUE - b : a < Integer.MIN_VALUE - b ) {
    throw new ArithmeticException("Not in range");
  }

  return a + b;
}

public void setValues(int a, int b) {
  this.a = a;
  this.b = b;
}
{code}

The {{
The getSum()
...
 method 
...
 may 
...
 return 
...
 a 
...
 different 
...
 sum 
...
 every 
...
 time 
...
 it 
...
 is 
...
 invoked 
...
 from 
...
 different 
...
 threads. 
...
 For 
...
 instance, 
...
 if 
...
a
...
 and 
...
b
...
 currently 
...
 have 
...
 the 
...
 value 
...
 0, 
...
 and 
...
 one 
...
 thread 
...
 calls 
...
getSum()
...
 while 
...
 another 
...
 calls 
...
setValues(1, 
...
 1)
...
, 
...
 then 
...
getSum()
...
 might 
...
 return 
...
 0, 
...
 1, 
...
 or 
...
 2. 
...
 Of 
...
 these, 
...
 the 
...
 value 
...
 1 
...
 is 
...
 unacceptable; 
...
 it 
...
 is 
...
 returned 
...
 when 
...
 the 
...
 first 
...
 thread 
...
 reads 
...
a
...
 and 
...
b
...
, 
...
 after 
...
 the 
...
 second 
...
 thread 
...
 has 
...
 set 
...
 the 
...
 value 
...
 of 
...
a
...
 but 
...
 before 
...
 it 
...
 has 
...
 set 
...
 the 
...
 value 
...
 of 
...
b
...
.
Noncompliant Code Example (addition, 
...
 atomic 
...
 integer 
...
 fields)
...
The 
...
 issues 
...
 described 
...
 in 
...
 the 
...
 previous 
...
 noncompliant 
...
 code 
...
 example 
...
 can 
...
 also 
...
 arise 
...
 when 
...
 the 
...
 volatile 
...
 variables 
...
a
...
 and 
...
b
...
 are 
...
 replaced 
...
 with 
...
 atomic 
...
 integers. 
 Code Block
bgColor#FFcccc
 

{code:bgColor=#FFcccc}
private final AtomicInteger a = new AtomicInteger();
private final AtomicInteger b = new AtomicInteger();
  
public int getSum() throws ArithmeticException {

  // Check for integer overflow
  if( b.get() > 0 ? a.get() > Integer.MAX_VALUE - b.get() : a.get() < Integer.MIN_VALUE - b.get() ) {
    throw new ArithmeticException("Not in range");
  }
  return a.get() + b.get(); // or, return a.getAndAdd(b.get());
}

public void setValues(int a, int b) {
  this.a.set(a);
  this.b.set(b);
}
{code}


For 
...
 example, 
...
 when 
...
 a 
...
 thread 
...
 is 
...
 executing 
...
setValues()
...
 another 
...
 may 
...
 invoke 
...
getSum()
...
 and 
...
 retrieve 
...
 an 
...
 incorrect 
...
 result. 
...
 Furthermore, 
...
 in 
...
 the 
...
 absence 
...
 of 
...
 synchronization, 
...
 there 
...
 are 
...
 data 
...
 races 
...
 in 
...
 the 
...
 check 
...
 for 
...
integer overflow.
Compliant Solution (addition)
...
This 
...
 compliant 
...
 solution 
...
 synchronizes 
...
 the 
...
setValues()
...
 method 
...
 so 
...
 that 
...
 the 
...
 entire 
...
 operation 
...
 is 
...
 atomic.


{:=
 Code Block
bgColor
#ccccff
}
private int a;
private int b;

public synchronized int getSum() throws ArithmeticException {
  // Check for integer overflow
  if( b > 0 ? a > Integer.MAX_VALUE - b : a < Integer.MIN_VALUE - b ) {
    throw new ArithmeticException("Not in range");
  }

  return a + b;
}

public synchronized void setValues(int a, int b) {
  this.a = a;
  this.b = b;
}
{code}


Unlike 
...
 the 
...
 noncompliant 
...
 code 
...
 example, 
...
 if 
...
a
...
 and 
...
b
...
 currently 
...
 have 
...
 the 
...
 value 
...
 0, 
...
 and 
...
 one 
...
 thread 
...
 calls 
...
getSum()
...
 while 
...
 another 
...
 calls 
...
setValues(1, 
...
 1)
...
, 
...
getSum()
...
 may 
...
 return 
...
 return 
...
 0, 
...
 or 
...
 2, 
...
 depending 
...
 on 
...
 which 
...
 thread 
...
 obtains 
...
 the 
...
 intrinsic 
...
 lock 
...
 first. 
...
 The 
...
 locking 
...
 guarantees 
...
 that 
...
getSum()
...
 will 
...
 never 
...
 return 
...
 the 
...
 unacceptable 
...
 value 
...
 1.
...
Risk 
...
 Assessment
...
If 
...
 operations 
...
 on 
...
 shared 
...
 variables 
...
 are 
...
 not 
...
 atomic, 
...
 unexpected 
...
 results 
...
 may 
...
 be 
...
 produced. 
...
 For 
...
 example, 
...
 there 
...
 can 
...
 be 
...
 inadvertent 
...
 information 
...
 disclosure 
...
 as 
...
 one 
...
 user 
...
 may 
...
 be 
...
 able 
...
 to 
...
 receive 
...
 information 
...
 about 
...
 other 
...
 users.
...
 Rule 
 Severity 
 Likelihood 
 Remediation Cost 
 Priority 
 Level 
 CON01- J 
 medium 
 probable 
 medium 
 P8 
 L2 
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
 Wiki Markup
\[[API 06|AA. Java References#API 06]\] Class AtomicInteger
\[[JLS 05|AA. Java References#JLS 05]\] [Chapter 17, Threads and Locks|http://java.sun.com/docs/books/jls/third_edition/html/memory.html], section 17.4.5 Happens-before Order, section 17.4.3 Programs and Program Order, section 17.4.8 Executions and Causality Requirements
\[[Tutorials 08|AA. Java References#Tutorials 08]\] [Java Concurrency Tutorial|http://java.sun.com/docs/books/tutorial/essential/concurrency/index.html]
\[[Lea 00|AA. Java References#Lea 00]\] Sections, 2.2.7 The Java Memory Model, 2.2.5 Deadlock, 2.1.1.1 Objects and locks
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 66: Synchronize access to shared mutable data
\[[Daconta 03|AA. Java References#Daconta 03]\] Item 31: Instance Variables in Servlets
\[[JavaThreads 04|AA. Java References#JavaThreads 04]\] Section 5.2 Atomic Variables
\[[Goetz 06|AA. Java References#Goetz 06]\] 2.3. "Locking"
\[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 667|http://cwe.mitre.org/data/definitions/667.html] "Insufficient Locking", [CWE ID 413|http://cwe.mitre.org/data/definitions/413.html] "Insufficient Resource Locking", [CWE ID 366|http://cwe.mitre.org/data/definitions/366.html] "Race Condition within a Thread", [CWE ID 567|http://cwe.mitre.org/data/definitions/567.html
...
]
...
 "Unsynchronized Access to Shared Data"
...
11. Concurrency (CON)      11. Concurrency (CON)      CON02-J. Always synchronize on the appropriate object