Mismatched Reader Writer Lock Release

ID

csharp.no_write_lock_release_on_read_lock

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

concurrency, deadlock, locking, reliability

Description

Reports a reader/writer lock released with the method that does not match how it was acquired — a write lock exited with ExitReadLock / ReleaseReaderLock, or a read lock exited with ExitWriteLock / ReleaseWriterLock. The release throws, and the lock that was really taken is never given back.

Rationale

ReaderWriterLock and ReaderWriterLockSlim track the reader and the writer side of the lock independently — separate recursion counts, separate wait queues — so the release methods are not interchangeable names for one operation. Calling the wrong one is not a style problem: the lock implementation checks that the calling thread actually holds the kind of lock being released and raises SynchronizationLockException when it does not.

The damage compounds because of where the exception lands. The release is usually the last statement of the guarded section, often inside a finally. Throwing there means the real lock is still held, there is no second release attempt on the way out, and the recursion count never returns to zero. From that moment every thread that asks for the resource — readers and writers alike — waits on a lock that nobody will ever release. One mismatched call takes the whole shared resource out of service, and the stack trace points at the release rather than at the design mistake.

The pairs must correspond exactly:

  • write side — AcquireWriterLock, UpgradeToWriterLock, EnterWriteLock, TryEnterWriteLock release with ReleaseWriterLock / ExitWriteLock;

  • read side — AcquireReaderLock, DowngradeFromWriterLock, EnterReadLock, TryEnterReadLock release with ReleaseReaderLock / ExitReadLock.

using System.Threading;

public class Catalog
{
    public void Rebuild(ReaderWriterLockSlim guard)
    {
        guard.EnterWriteLock();
        try
        {
            Refresh();
        }
        finally
        {
            guard.ExitReadLock();       // FLAW — write lock exited on the read side
        }
    }

    public void RebuildCorrectly(ReaderWriterLockSlim guard)
    {
        guard.EnterWriteLock();
        try
        {
            Refresh();
        }
        finally
        {
            guard.ExitWriteLock();      // OK — matched pair
        }
    }

    private void Refresh() { }
}

Remediation

Release on the same side the lock was taken. Enter/exit and acquire/release calls should read as mirror images of each other within one method, with the release in a finally so it also runs on the exception path.

Two ways to make the mistake unrepresentable:

  • wrap the acquisition in a small IDisposable scope (using var scope = guard.Read();) that remembers which side it took and exits that same side on Dispose;

  • upgrade a read lock with EnterUpgradeableReadLock rather than juggling two separate locks — its exit method, ExitUpgradeableReadLock, is unambiguous, and the nested write lock keeps its own matched EnterWriteLock / ExitWriteLock pair.

For new code, prefer ReaderWriterLockSlim over the legacy ReaderWriterLock, and consider whether a plain lock is enough: a reader/writer lock only pays off when reads greatly outnumber writes and the guarded section is long.

References