CodeQL documentation
CodeQL resources

Sleep with lock held

ID: java/sleep-with-lock-held
Kind: problem
Security severity: 
Severity: error
Precision: medium
Tags:
   - reliability
   - correctness
   - concurrency
   - external/cwe/cwe-833
Query suites:
   - java-security-and-quality.qls

Click to see the query in the CodeQL repository

Calling Thread.sleep with a lock held may lead to very poor performance or even deadlock. This is because Thread.sleep does not cause a thread to release its locks.

Recommendation

Thread.sleep should be called only outside of a synchronized block. However, a better way for threads to yield execution time to other threads may be to use either of the following solutions:

  • The java.util.concurrent library

  • The wait and notifyAll methods

Example

In the following example of the problem, two threads, StorageThread and OtherThread, are started. Both threads output a message to show that they have started but then StorageThread locks counter and goes to sleep. The lock prevents OtherThread from locking counter, so it has to wait until StorageThread has woken up and unlocked counter before it can continue.

class StorageThread implements Runnable{
    public static Integer counter = 0;
    private static final Object LOCK = new Object();

    public void run() {
        System.out.println("StorageThread started.");
        synchronized(LOCK) {  // "LOCK" is locked just before the thread goes to sleep
            try {
                Thread.sleep(5000);
            } catch (InterruptedException e) { ... }
        }
        System.out.println("StorageThread exited.");
    }
}

class OtherThread implements Runnable{
    public void run() {
        System.out.println("OtherThread started.");
        synchronized(StorageThread.LOCK) {
            StorageThread.counter++;
        }
        System.out.println("OtherThread exited.");
    }
}

public class SleepWithLock {
    public static void main(String[] args) {
        new Thread(new StorageThread()).start();
        new Thread(new OtherThread()).start();
    }
}

To avoid this problem, StorageThread should call Thread.sleep outside the synchronized block instead, so that counter is unlocked.

References

  • © GitHub, Inc.
  • Terms
  • Privacy