Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: fixed CSs with post-increment check for integer overflow

...

Code Block
bgColor#FFcccc
private int itemsInInventory = 100;

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

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

For example, if the removeItem() method is concurrently invoked by two threads, the execution of these threads may be interleaved so that:

...

Code Block
bgColor#FFcccc
private volatile int itemsInInventory = 100;

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

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

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.

...

Code Block
bgColor#ccccff
public final int returnItem() {
  int temp;   = itemsInInventory.getAndIncrement();
  if( (temp = itemsInInventory.get()) == Integer.MAXMIN_VALUE) { // Check for integer overflow
    return -1; // Error Code
  }

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

The getAndIncrement() does not check for integer overflow. Consequently, returnItem() has to check the returned value to ensure that itemsInInventory has not wrapped around to Integer.MIN_VALUE after the increment operation. This can be done after performing the getAndIncrement() operation.

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. Notably, this functionality could also be implemented by using the compareAndSet() method. The getAndIncrement() alternative is useful when control over setting the returned value must lie in the hands of the caller instead of the invoked method (returnItem()).

...

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 
}

public synchronized final synchronized int returnItem() {
  if (itemsInInventory == Integer.MAXMIN_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).

...

Code Block
bgColor#ccccff
public final int returnItem() {
  synchronized(this) {
    if (itemsInInventory == Integer.MAXMIN_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).

...

Code Block
bgColor#ccccff
public int returnItem() {
  Boolean myLock = false;
    
  try {
    myLock = lock.tryLock(); 
    	
    if (itemsInInventory == Integer.MAXMIN_VALUE) { // Check for integer overflow
      return -1; 
    } else {
      return itemsInInventory++;
    }  
  } finally {
    if (myLock) {
      lock.unlock();
    }
  }
  return -1; // Error code
}

...