CodeQL documentation
CodeQL resources

A lock is held during a wait

ID: cs/locked-wait
Kind: problem
Security severity: 
Severity: warning
Precision: high
Tags:
   - reliability
   - correctness
   - concurrency
   - external/cwe/cwe-662
   - external/cwe/cwe-833
Query suites:
   - csharp-security-and-quality.qls

Click to see the query in the CodeQL repository

Holding a lock during a call to System.Threading.Monitor.Wait() can lead to performance problems or deadlock because the lock can prevent other threads from running. This can be caused by nesting the call to System.Threading.Monitor.Wait() in two lock statements, or by waiting on a different object to the one which is locked.

Synchronized methods (with the attribute [MethodImpl(MethodImplOptions.Synchronized)]) can also cause problems, because they are equivalent to lock(this) or lock(typeof(Class)).

Recommendation

Ensure that the call to System.Threading.Monitor.Wait() occurs within a single lock statement and ensure that the argument to System.Threading.Monitor.Wait() matches the variable in the lock statement.

Example

The following example locks two variables, countLock and textLock, then calls System.Threading.Monitor.Wait(). However for the duration of Wait(), the variable countLock is locked, which deadlocks the program.

class Message
{
    readonly Object countLock = new Object();
    readonly Object textLock = new Object();

    int count;
    string text;

    public void Print()
    {
        lock (countLock)
        {
            lock (textLock)
            {
                while (text == null)
                    System.Threading.Monitor.Wait(textLock);
                System.Console.Out.WriteLine(text + "=" + count);
            }
        }
    }

    public string Text
    {
        set
        {
            lock (countLock)
            {
                lock (textLock)
                {
                    ++count;
                    text = value;
                    System.Threading.Monitor.Pulse(textLock);
                }
            }
        }
    }
}

The problem can be fixed by moving the lock statement so that countLock is not locked for the duration of the wait.

class Message
{
    readonly Object countLock = new Object();
    readonly Object textLock = new Object();

    int count;
    string text;

    public void Print()
    {
        lock (textLock)
        {
            while (text == null)
                System.Threading.Monitor.Wait(textLock);
            lock (countLock)
                System.Console.Out.WriteLine(text + "=" + count);
        }
    }

    public string Text
    {
        set
        {
            lock (countLock)
            {
                lock (textLock)
                {
                    ++count;
                    text = value;
                    System.Threading.Monitor.Pulse(textLock);
                }
            }
        }
    }
}

References

  • © GitHub, Inc.
  • Terms
  • Privacy