10

I'm using ReaderWriterLockSlim to guard some operations. I would like to favor readers over writers, so that when a reader holds the lock for long and a writer is attempting to acquire the write lock, further readers down the road are not blocked by the writer's attempt (which is what would happen instead if the writer was blocked on lock.EnterWriteLock()).

To this end, I though that the writer could use TryEnterWriteLock with a short timeout in a loop, so that subsequent readers would still be able to acquire the read lock while the writer can't. However, to my surprise, I found out that an unsuccessful call to TryEnterWriteLock changes the state of the lock, blocking future readers anyway. Proof of concept code:

System.Threading.ReaderWriterLockSlim myLock = new System.Threading.ReaderWriterLockSlim(System.Threading.LockRecursionPolicy.NoRecursion);

System.Threading.Thread t1 = new System.Threading.Thread(() =>
{
    Console.WriteLine("T1:{0}: entering read lock...", DateTime.Now);
    myLock.EnterReadLock();
    Console.WriteLine("T1:{0}: ...entered read lock.", DateTime.Now);

    System.Threading.Thread.Sleep(10000);
});

System.Threading.Thread t2 = new System.Threading.Thread(() =>
{
    System.Threading.Thread.Sleep(1000);

    while (true)
    {
        Console.WriteLine("T2:{0}: try-entering write lock...", DateTime.Now);
        bool result = myLock.TryEnterWriteLock(TimeSpan.FromMilliseconds(1500));
        Console.WriteLine("T2:{0}: ...try-entered write lock, result={1}.", DateTime.Now, result);

        if (result)
        {
            // Got it!
            break;
        }

        System.Threading.Thread.Yield();
    }

    System.Threading.Thread.Sleep(9000);
});

System.Threading.Thread t3 = new System.Threading.Thread(() =>
{
    System.Threading.Thread.Sleep(2000);

    Console.WriteLine("T3:{0}: entering read lock...", DateTime.Now);
    myLock.EnterReadLock();
    Console.WriteLine("T3:{0}: ...entered read lock!!!!!!!!!!!!!!!!!!!", DateTime.Now);

    System.Threading.Thread.Sleep(8000);
});

The output of this code is:

T1:18-09-2015 16:29:49: entering read lock...
T1:18-09-2015 16:29:49: ...entered read lock.
T2:18-09-2015 16:29:50: try-entering write lock...
T3:18-09-2015 16:29:51: entering read lock...
T2:18-09-2015 16:29:51: ...try-entered write lock, result=False.
T2:18-09-2015 16:29:51: try-entering write lock...
T2:18-09-2015 16:29:53: ...try-entered write lock, result=False.
T2:18-09-2015 16:29:53: try-entering write lock...
T2:18-09-2015 16:29:54: ...try-entered write lock, result=False.
T2:18-09-2015 16:29:54: try-entering write lock...
T2:18-09-2015 16:29:56: ...try-entered write lock, result=False.
T2:18-09-2015 16:29:56: try-entering write lock...
T2:18-09-2015 16:29:57: ...try-entered write lock, result=False.
T2:18-09-2015 16:29:57: try-entering write lock...
T2:18-09-2015 16:29:59: ...try-entered write lock, result=False.
T2:18-09-2015 16:29:59: try-entering write lock...

As you can see, even though thread 2 (the "Writer") hasn't acquired a writer lock and it's not in an EnterWriteLock call, thread 3 gets blocked for good. I can see a similar behavior with ReaderWriterLock.

Am I doing anything wrong? If not, what options do I have to favor readers when a writer is queued?

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
Gabriele Giuseppini
  • 1,541
  • 11
  • 19
  • 2
    Interesting. It seems that if you use `TryEnterWriteLock(0)` instead (and move the timeout to `Sleep` somewhere below), it works fine (here, at least). However, by my reading of the [documentation](https://msdn.microsoft.com/en-us/library/bb339812.aspx), it should not behave like that: _“additional threads that try to enter read mode or upgradeable mode block until all the threads waiting to enter write mode have either timed out or entered write mode and then exited from it”_. – Mormegil Sep 18 '15 at 14:51
  • Add loop to the thread `t3` and you will have success. `t3` tries to occur read lock when `t2` is trying to occur write look with timeout. – Hamlet Hakobyan Sep 18 '15 at 14:57
  • Yes Mormegil, exactly - the documentation hints at absolutely different behavior.... – Gabriele Giuseppini Sep 18 '15 at 14:58
  • 1
    @Mormegil It behaves as stated in documentation. `t3` tries to occur read lock when `t2` waits by timeout. – Hamlet Hakobyan Sep 18 '15 at 15:02
  • 1
    @Hamlet, t2 times out, so according to the documentation, the "additional threads that try to enter read mode" should be unblocked, but they are not. – Gabriele Giuseppini Sep 18 '15 at 15:05
  • @Hamlet, changing t3 to also use `TryEnterReadLock` in a loop doesn't seem to work, either. t3 never gets the read lock. – Gabriele Giuseppini Sep 18 '15 at 15:08
  • Additional thread doesn't TRY (you have used `EnterReadLock` instead of `TryEnterReadLock(int)`). Change `Thread.Sleep` for `t3` to 3000 or use `TryEnterReadLock(int)` and you will see difference. – Hamlet Hakobyan Sep 18 '15 at 15:09
  • @Mormegil, thanks for the zero timeout suggestion, that seems to work. I guess I could build a loop around that, but I'm afraid that writers would starve way frequently as they need to hope that no one holds the lock each time they poll for it... – Gabriele Giuseppini Sep 18 '15 at 15:11
  • 2
    Change Thread.Sleep for t3 to 3000 will not work because you are trying to occur writelook in loop. But using TryEnterReadLock(int) will solve your problem. – Hamlet Hakobyan Sep 18 '15 at 15:21
  • @Hamlet, thanks, it seems now that if also t3 uses `TryEnterReadLock`, then it's able to acquire the read lock while t2 is still polling for it. Nice. – Gabriele Giuseppini Sep 18 '15 at 16:03

2 Answers2

3

I can’t help but I believe this is a .NET Framework bug (UPDATE: I have reported the bug). The following straightforward program (a simplified version of the above) illustrates that:

var myLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);

var t1 = new Thread(() =>
{
    Console.WriteLine("T1:{0}: entering read lock...", DateTime.Now);
    myLock.EnterReadLock();
    Console.WriteLine("T1:{0}: ...entered read lock.", DateTime.Now);

    Thread.Sleep(50000);

    Console.WriteLine("T1:{0}: exiting", DateTime.Now);
    myLock.ExitReadLock();
});

var t2 = new Thread(() =>
{
    Thread.Sleep(1000);

    Console.WriteLine("T2:{0}: try-entering write lock...", DateTime.Now);
    bool result = myLock.TryEnterWriteLock(3000);
    Console.WriteLine("T2:{0}: ...try-entered write lock, result={1}.", DateTime.Now, result);

    Thread.Sleep(50000);

    if (result)
    {
        myLock.ExitWriteLock();
    }
    Console.WriteLine("T2:{0}: exiting", DateTime.Now);
});

var t3 = new Thread(() =>
{
    Thread.Sleep(2000);

    Console.WriteLine("T3:{0}: entering read lock...", DateTime.Now);
    myLock.EnterReadLock();
    Console.WriteLine("T3:{0}: ...entered read lock!!!!!!!!!!!!!!!!!!!", DateTime.Now);

    Thread.Sleep(50000);

    myLock.ExitReadLock();
    Console.WriteLine("T3:{0}: exiting", DateTime.Now);
});

t1.Start();
t2.Start();
t3.Start();

t1.Join();
t2.Join();
t3.Join();

The following happens in a simple order, no lock convoys, no races, no loops or anything.

  1. T1 acquires a read lock.
  2. T2 tries to acquire a write lock and blocks, waiting for a timeout (as T1 holds the lock).
  3. T3 tries to acquire a read lock and blocks (because T2 is blocked waiting for the write lock, and per the documentation, this means all further readers are blocked until timeouts).
  4. T2’s timeout expires. Per the documentation, T3 should now wake up and acquire the read lock. However, this does not happen and T3 is blocked forever (or, in the case of this example, for those 50 seconds until T1 leaves the read lock).

AFAICT, the ExitMyLock in ReaderWriterLockSlim’s WaitOnEvent should have been ExitAndWakeUpAppropriateWaiters.

Mormegil
  • 7,955
  • 4
  • 42
  • 77
  • Can you state where the behavior does not meet documentation? – Hamlet Hakobyan Sep 18 '15 at 16:15
  • 3
    Well, could you explain why such a program (its `T3` thread) should block indefinitely? What is it waiting for what and why? Also, I have quoted the documentation above: _“block until all the threads waiting to enter write mode have either timed out or entered write mode and then exited from it”_. They have timed out, but the thread is still blocked. – Mormegil Sep 18 '15 at 16:20
  • Not so sure "bug" is the appropriate word. "Not optimal" is more like it, there's some concurrency opportunity loss. It can only really go wrong in a program that never releases a read lock, that's a buggy program. Otherwise the reason nobody ever notices this. ReaderWriterLock (not slim) does the exact same thing btw. – Hans Passant Sep 18 '15 at 17:36
  • 2
    @Hans: If it's not a bug, then the documentation is misleading. The way it's written, it makes it sound like there is no difference between a thread that times out asking for a write lock, or a thread that acquires the write lock and then exits. In both cases, you expect that any queued read requests will unblock immediately. And this does happen in one case, but not the other. I agree with Mormegil, that a call to `ExitAndWakeUpAppropriateWaiters` may have been missed. – sstan Sep 18 '15 at 17:47
  • 1
    Report this on the corefx Github issue tracker. – usr Sep 21 '15 at 15:57
  • Is this "behaviour" still in effect? – g.pickardou Jul 04 '17 at 10:31
  • See the [above-linked issue](https://github.com/dotnet/corefx/issues/3364): the bug in `ReaderWriterLockSlim` was fixed; I am unsure in which releases the bug fix shipped, but in my current .NET Core project, it seems to work correctly. (Note non-slim `ReaderWriterLock` is still broken and won’t be fixed.) – Mormegil Jul 17 '17 at 11:35
2

I agree with Mormegil's answer that your observed behavior doesn't seem to comply with what the documentation for ReaderWriterLockSlim.TryEnterWriteLock Method (Int32) states:

While threads are blocked waiting to enter write mode, additional threads that try to enter read mode or upgradeable mode block until all the threads waiting to enter write mode have either timed out or entered write mode and then exited from it.

Notice that there are 2 documented ways that other threads waiting to enter read mode will stop waiting:

  • TryEnterWriteLock times out.
  • TryEnterWriteLock succeeds, and then releases the lock by calling ExitWriteLock.

The 2nd scenario works as expected. But the 1st (the timeout scenario) doesn't, as your code sample clearly shows.

Why does it not work?

Why do threads trying to enter read mode keep waiting, even after an attempt to enter write mode times out?

Because, in order to allow waiting threads to enter read mode, 2 things need to happen. The thread that times out waiting for a write lock needs to:

The above is what should happen. But in practice, when TryEnterWriteLock times out, only the first step is performed (resetting the flag), but not the 2nd (signaling waiting threads to retry acquiring the lock). As a result, in your case, the thread trying to acquire the read lock keeps waiting indefinitely because it never gets "told" that it should wake up and check the flag again.

As pointed out by Mormegil, changing the call from ExitMyLock() to ExitAndWakeUpAppropriateWaiters() in this line of code appears to be all that's needed to fix the problem. Because the fix seems so simple, I am also inclined to think that this is just a bug that was overlooked.

How is this information useful?

Understanding the cause allows us to realize that the "buggy" behavior's impact is somewhat limited in scope. It only causes threads to block "indefinitely" if they attempt to acquire the lock after some thread invokes TryEnterWriteLock, but before the TryEnterWriteLock invocation times out. And it doesn't really block indefinitely. It does eventually unblock as soon as some other thread releases its lock normally, which would be the expected scenario in this case.

This also means that any thread attempting to enter READ mode after TryEnterWriteLock times out will be able to do so without any problems.

To illustrate this, run the following code snippet:

private static ReaderWriterLockSlim rwLock = new ReaderWriterLockSlim();
private static Stopwatch stopwatch = new Stopwatch();

static void Log(string logString)
{
    Console.WriteLine($"{(long)stopwatch.Elapsed.TotalMilliseconds:D5}: {logString}");
}

static void Main(string[] args)
{
    stopwatch.Start();

    // T1 - Initial reader
    new Thread(() =>
    {
        Log("T1 trying to enter READ mode...");
        rwLock.EnterReadLock();
        Log("T1 entered READ mode successfully!");
        Thread.Sleep(10000);
        rwLock.ExitReadLock();
        Log("T1 exited READ mode.");
    }).Start();

    // T2 - Writer times out.
    new Thread(() =>
    {
        Thread.Sleep(1000);
        Log("T2 trying to enter WRITE mode...");
        if (rwLock.TryEnterWriteLock(2000))
        {
            Log("T2 entered WRITE mode successfully!");
            rwLock.ExitWriteLock();
            Log("T2 exited WRITE mode.");
        }
        else
        {
            Log("T2 timed out! Unable to enter WRITE mode.");
        }
    }).Start();

    // T3 - Reader blocks longer than it should, BUT...
    //      is eventually unblocked by T4's ExitReadLock().
    new Thread(() =>
    {
        Thread.Sleep(2000);
        Log("T3 trying to enter READ mode...");
        rwLock.EnterReadLock();
        Log("T3 entered READ mode after all!  T4's ExitReadLock() unblocked me.");
        rwLock.ExitReadLock();
        Log("T3 exited READ mode.");
    }).Start();

    // T4 - Because the read attempt happens AFTER T2's timeout, it doesn't block.
    //      Also, once it exits READ mode, it unblocks T3!
    new Thread(() =>
    {
        Thread.Sleep(4000);
        Log("T4 trying to enter READ mode...");
        rwLock.EnterReadLock();
        Log("T4 entered READ mode successfully! Was not affected by T2's timeout \"bug\"");
        rwLock.ExitReadLock();
        Log("T4 exited READ mode. (Side effect: wakes up any other waiter threads)");
    }).Start();
}

Output:

00000: T1 trying to enter READ mode...
00001: T1 entered READ mode successfully!
01011: T2 trying to enter WRITE mode...
02010: T3 trying to enter READ mode...
03010: T2 timed out! Unable to enter WRITE mode.
04013: T4 trying to enter READ mode...
04013: T4 entered READ mode successfully! Was not affected by T2's timeout "bug"
04013: T4 exited READ mode. (Side effect: wakes up any other waiter threads)
04013: T3 entered READ mode after all! T4's ExitReadLock() unblocked me.
04013: T3 exited READ mode.
10005: T1 exited READ mode.

Also note that, the T4 reader thread was not strictly required in order to unblock T3. T1's eventual ExitReadLock would also have unblocked T3.

Final thoughts

Though the current behavior is less than ideal, and does feel like a bug in the .NET library, in a real life scenario, the reader(s) thread(s) that caused the writer thread to timeout in the first place would eventually exit READ mode. And that in turn would unblock any waiting reader threads that may be stuck because of the "bug". So the real life impact of the "bug" should be minimal.

Still, as suggested in the comments, to make your code even more reliable, it wouldn't be a bad idea to change your EnterReadLock() invocation to TryEnterReadLock with a reasonable timeout value and invoked inside a loop. That way, you can have some measure of control over the maximum amount of time that a reader thread will stay "stuck".

sstan
  • 35,425
  • 6
  • 48
  • 66