c# ReaderWriterLockSlim corrupt after interrupt thread that invokes ExitReadLock

Question

I encountered a scenario where ReaderWriterLockSlim seems to get broken after a series of legal actions.

The flow is:

Thread 1 takes writer lock1
Thread 2 tries to take reader lock1 - blocks
Thread 2 is interrupted, and calls lock1.ExitReadLock Thread 2 didn't get the lock. It seems that an exception should have been thrown.
Thread 1 exits writer lock of lock1
Any thread that tries to take lock1.EnterReadLock will block forever

After stage 3 above, a debugger shows that lock1.CurrentReadCount is corrupt - seems to have overflowed down to 0x7FFFFFF.

I wonder if anyone had encountered this, or maybe I'm missing something.

The code that reproduces it:

[TestMethod]
public void ReproTest()
{
    var rwlock = new ReaderWriterLockSlim();
    rwlock.EnterWriteLock();

    bool taken = false;
    var reader = new Thread(() =>
    {
        try
        {
            rwlock.EnterReadLock();
            s_logger.Info("Enter");

        }
        catch (ThreadInterruptedException)
        {
            rwlock.ExitReadLock();
        }
    });
    reader.Name = "Reader";
    reader.Start();
    Thread.Sleep(1000);

    reader.Interrupt();

    Thread.Sleep(1000);

    rwlock.ExitWriteLock();

    while (!taken)
    {
        taken = rwlock.TryEnterReadLock(1000);
    }

    Thread.Sleep(1000);
}

The second exception `SynchronizationLockException` *is* thrown at the line `rwlock.ExitReadLock()` when I try this. — Matthew Watson, Mar 17 '13 at 14:17

score 1 · Answer 1 · answered Mar 17 '13 at 13:59

1

The reader is never entering the read lock. It sits waiting for the write to be released. When it's interrupted, you then try to exit even though you never entered, causing read count to go below 0 I suppose :)

answered Mar 17 '13 at 13:59

Jeremy Rosenberg

792
6
18

Actually, it will throw another exception: `SynchronizationLockException` with message "*The read lock is being released without being held*". It shouldn't corrupt the lock object. – Matthew Watson Mar 17 '13 at 14:13

score 1 · Accepted Answer · answered Mar 17 '13 at 17:40

This looks like a bug in the framework (tested on v3.5 and v4.0). The ExitReadLock() should throw a SynchronizationLockException, but doesn't in this case. Indeed, you can trigger a very similar issue much more simply with the following:

rwlock.EnterReadLock();
rwlock.ExitReadLock();
// This should throw a SynchronizationLockException but doesn't
rwlock.ExitReadLock();
// At this point, rwlock.CurrentReaderCount = 0x0fffffff

(In fact, ExitReadLock() will corrupt the lock if it's called without a matching EnterReadLock() on any thread that has previously entered the lock.)

The issue only occurs when the ReaderWriterLockSlim is created using the parameterless constructor, or with LockRecursionPolicy.NoRecursion. If created with LockRecursionPolicy.SupportsRecursion, it will not be corrupted by the unmatched ExitReadLock().

If you expect the reader thread to be interrupted whilst waiting for entry into lock, I would suggest changing the reader thread method to:

var reader = new Thread(() =>
{
    var entered = false;
    try
    {
        rwlock.EnterReadLock();
        entered = true;
        s_logger.Info("Enter");
    }
    finally
    {
        if (entered) rwlock.ExitReadLock();
    }
});

Simpler solution is to execute rwlock.EnterReadLock() above the try block and avoid the 'entered' variable completely. Both of the approaches though carry the same small risk: deadlock when asynchronous exception happens just after the rwlock.EnterReadLock() line. In my proposal the finally block will not get executed; in the code above 'entered' will still be false. — Slobodan Savkovic, Feb 27 '15 at 19:26

score 0 · Answer 3 · answered Mar 17 '13 at 14:18

Code that fixes what @Lasse and @Jeremy have pointed out:

static public void ReproTest()
{
    var rwlock = new ReaderWriterLockSlim();
    rwlock.EnterWriteLock();
    s_logger.Info("0:Enter");

    bool taken1 = false;
    var reader = new Thread(() =>
    {
        try
        {
            rwlock.EnterReadLock();
            s_logger.Info("1:Enter");
            // only set to true if taken
            taken1 = true;
        }
        catch (ThreadInterruptedException)
        {
            // only release if taken
            if (taken1)
                rwlock.ExitReadLock();
            taken1 = false;
        }
    });
    reader.Name = "Reader";
    reader.Start();
    Thread.Sleep(1000);

    reader.Interrupt();

    Thread.Sleep(1000);

    rwlock.ExitWriteLock();

    // 2nd taken variable here only so we can see state of taken1
    bool taken2 = taken1;
    while (!taken2)
    {
        taken2 = rwlock.TryEnterReadLock(1000);
        s_logger.Info("2:Enter");
    }

    Thread.Sleep(1000);
}

When run, the debug output correctly shows the write lock being taken, the 1st read lock NOT taken, and the 2nd read lock taken:

0:Enter
A first chance exception of type 'System.Threading.ThreadInterruptedException' occurred in mscorlib.dll
The thread 'Reader' (0x1358) has exited with code 0 (0x0).
2:Enter

The code is still messed up though. If there is no exception, `ExitReadLock()` will never be called. Or if there is an exception other than `ThreadInterruptedException`, then `ExitReadLock()` will also not be called. Really, there should be no try/catch at all, rather a try/finally *after* a successful call to `EnterReadLock()`, where the `finally` calls `ExitReadLock()`. But this is all test code anyway. — Matthew Watson, Mar 17 '13 at 14:23

c# ReaderWriterLockSlim corrupt after interrupt thread that invokes ExitReadLock

3 Answers3