Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: added getAndIncrement examples to NCEs/CSs

...

As a result, a decrement operation is "lost" and the itemsInInventory value is now incorrect.

Similarly, a returnItem() method that increments itemsInInventory is also nonatomic:

Code Block
bgColor#FFcccc

public final int returnItem() {
  itemsInInventory++;
}

Noncompliant Code Example (volatile)

...

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

Similarly, the returnItem() method does not perform the increment operation atomically.

Code Block
bgColor#FFcccc

public final int returnItem() {
  itemsInInventory++;
}

Compliant Solution (java.util.concurrent.atomic classes)Compliant Solution (java.util.concurrent.atomic classes)

This compliant solution uses a java.util.concurrent.atomic.AtomicInteger variable which allows certain composite operations to be performed atomically.

...

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 returnItem(

...

)

...

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 method can be fixed by using the java.util.concurrent.atomic.AtomicInteger.getAndIncrement() method.

Code Block
bgColor#ccccff
privatepublic final int itemsInInventory = 100;

public synchronized int removeItemreturnItem() {
  ifint (itemsInInventorytemp; > 0) {
  if((temp  return itemsInInventory--; = itemsInInventory.get()) == Integer.MAX_VALUE) { // ReturnsCheck newfor count of items in inventory
  } 
integer overflow
    return -1; // Error Code
  }
}

Synchronization is more expensive than using the optimized java.util.concurrent utilities and should only be used when the utilities do not contain the facilities (methods) required to carry out the atomic operation. When synchronizing, 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 an alternative representation called block synchronization which synchronizes a block of code rather than a method, as highlighted in this compliant solution.

Code Block
bgColor#ccccff

private final int itemsInInventory = 100;

public int removeItem() {
  synchronized(this) {
    if (itemsInInventory > 0) {
      return itemsInInventory--;  // Returns new count of items in inventory    
    } 
    return -1; // Error code
  }
}

Block synchronization is preferable over method synchronization because it reduces the duration for which the lock is held and also protects against denial of service attacks. Block synchronization requires synchronizing on an internal private lock object instead of the intrinsic lock of the class's object (see CON04-J. Use the private lock object idiom instead of the Class object's intrinsic locking mechanism).

When the number of items is 0 most of the time, the synchronized block may be moved inside the if condition to reduce the performance cost associated with synchronization. In that case, the variable itemsInInventory must be declared as volatile because the check to determine whether it is greater than 0 should rely on the latest value of itemsInInventory.

Compliant Solution (ReentrantLock)

This compliant solution uses a java.util.concurrent.locks.ReentrantLock to atomically perform the post-decrement operation.


  if(temp == itemsInInventory.get()) {  
    return itemsInInventory.getAndIncrement();
  } else
    return -1;  // Error Code
  } 
}

It is necessary to use a local variable temp because there is a time-of-check to time-of-use (TOCTOU) condition between checking whether the inventory count is less than Integer.MAX_VALUE and using getAndIncrement() to increment it.

Compliant Solution (method synchronization)

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

private final int itemsInInventory = 100;

public final synchronized int removeItem() {
  if (itemsInInventory > 0) {
    return itemsInInventory--;  // Returns new count of items in inventory
  } 
  return -1; // Error Code 
}

similarly, the returnItem() method can be fixed by synchronizing it:

Code Block
bgColor#ccccff

public final synchronized int returnItem() {
  if(itemsInInventory == Integer.MAX_VALUE) { // Check for integer overflow
    return -1; // Error Code
  } else {
    return itemsInInventory++;
  }
}

Synchronization is more expensive than using the optimized java.util.concurrent utilities and should only be used when the utilities do not contain the facilities (methods) required to carry out the atomic operation. When synchronizing, 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 an alternative representation called block synchronization which synchronizes a block of code rather than a method, as highlighted in this compliant solution.

Code Block
bgColor#ccccff

private final int itemsInInventory = 100;

public int removeItem() {
  synchronized(this) {
    if (itemsInInventory > 0) {
      return itemsInInventory--;  // Returns new count of items in inventory    
    } 
    return -1; // Error code
  }
}

similarly, the returnItem() method can be fixed by using block synchronization:

Code Block
bgColor#ccccff

public final int returnItem() {
  synchronized(this) {
    if(itemsInInventory == Integer.MAX_VALUE) { // Check for integer overflow
      return -1; // Error Code
    } else {
      return itemsInInventory++;
    }
  }
}

Block synchronization is preferable over method synchronization because it reduces the duration for which the lock is held and also protects against denial of service attacks. Block synchronization requires synchronizing on an internal private lock object instead of the intrinsic lock of the class's object (see CON04-J. Use the private lock object idiom instead of the Class object's intrinsic locking mechanism).

When the number of items is 0 most of the time, the synchronized block may be moved inside the if condition to reduce the performance cost associated with synchronization. In that case, the variable itemsInInventory must be declared as volatile because the check to determine whether it is greater than 0 should rely on the latest value of itemsInInventory.

Compliant Solution (ReentrantLock)

This compliant solution uses a java.util.concurrent.locks.ReentrantLock to atomically perform the post-decrement operation.

Code Block
bgColor#ccccff

private int itemsInInventory = 100;
private final Lock lock = new ReentrantLock();
    
public int removeItem() {
  Boolean myLock = false;
    
  try {
    myLock = lock.tryLock(); 
    	
    if (itemsInInventory > 0) {
      return itemsInInventory--;
    }  
  } finally {
    if (myLock) {
      lock.unlock();
    }
  }
  return -1; // Error code
}

Similarly, the returnItem() method can be made atomic:

Code Block
bgColor#ccccff

public int returnItem
Code Block
bgColor#ccccff

private int itemsInInventory = 100;
private final Lock lock = new ReentrantLock();
    
public int removeItem() {
  Boolean myLock = false;
    
  try {
    myLock = lock.tryLock(); 
    	
    if (itemsInInventory > 0) == Integer.MAX_VALUE) { // Check for integer overflow
      return -1; 
    } else {
      return itemsInInventory--++;
    }  
  } finally {
    if (myLock) {
      lock.unlock();
    }
  }
  return -1; // Error code
}

...