CON15-J. Ensure actively held locks are released on exceptional conditions

An exceptional condition may circumvent the release of a lock. This can result in thread starvation and deadlock. According to the Java API [[API 06]], class ReentrantLock documentation:

A ReentrantLock is owned by the thread last successfully locking, but not yet unlocking it. A thread invoking lock will return, successfully acquiring the lock, when the lock is not owned by another thread.

This means that an unreleased lock in any thread will stop other threads from acquiring the same lock.

Noncompliant Code Example

This noncompliant code example protects a resource by using a ReentrantLock but on an exceptional condition, fails to release the lock. Control flow transfers to the catch block and the call to unlock() does not execute.

public void doSomething() {
  final Lock lock = new ReentrantLock();
  try {
    lock.lock();
    // Do something with the protected resource
    // This may cause an exception such as FileNotFoundException
    lock.unlock();
  } catch(FileNotFoundException fnf) {
    // Handle the exception
  }
}

Note that the lock is not released even when the doSomething() method returns.

Compliant Solution

This compliant solution uses a try-finally block immediately after acquiring the lock. This ensures that the lock is appropriately released even in the event of an exceptional condition.

public void doSomething() {
  final Lock lock = new ReentrantLock();
  lock.lock();
  try {
    // Do something with the protected resource
    // This may cause an exception such as FileNotFoundException
  } catch(FileNotFoundException fnf) {
    // Handle the exception
  } finally {
    lock.unlock();
  }
}

Risk Assessment

Failing to release a lock on an exceptional condition may lead to thread starvation and deadlock.

Automated Detection

TODO

Related Vulnerabilities

GERONIMO-2234

References

[[API 06]] Class ReentrantLock

VOID CON14-J. Ensure atomicity of 64-bit operations      11. Concurrency (CON)      CON16-J. Do not expect sleep() and yield() methods to have any synchronization semantics