C# ReaderWriterLockSlim Best Practice to Avoid Recursion

Question

I have a class using ReaderWriterLockSlim with a read method and a write method that uses the read method to retrieve the element to be modified. A quick example would be:

class FooLocker
{
    ReaderWriterLockSlim locker = new ReaderWriterLockSlim();
    List<Foo> fooList = new List<Foo>();

    public void ChangeFoo(int index, string bar)
    {
        locker.EnterWriteLock();

        try
        {
            Foo foo = GetFoo(index);
            foo.Bar = bar;
        }
        finally
        {
            locker.ExitWriteLock();
        }
    }

    public Foo GetFoo(int index) 
    {
        locker.EnterReadLock(); //throws System.Threading.LockRecursionException

        try
        {
            return fooList[index];
        }
        finally
        {
            locker.ExitReadLock();
        }
    }

    //snipped code for adding instances etc.
}

As above, this code throws a LockRecursionException when calling ChangeFoo() because a write lock is already held when GetFoo() tries to enter a read lock.

I've checked the documentation for ReaderWriterLockSlim and I can use LockRecursionPolicy.SupportsRecursion to allow the above to work. However, the documentation also recommends that this shouldn't be used for any new development and should only be used when upgrading existing code.

Given this, what is the best practice to achieve the same result where a write method can use a read-only method to retrieve the thing that needs to be modified?

did you try to check if the read lock is held by reading ReaderWriterLockSlim.IsReadLockHeld value? http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlockslim.isreadlockheld.aspx — Karel Frajták, Mar 28 '12 at 09:07
Setting SupportsRecursion wouldn't be the end of the world here, but Polity's anwer is the better approach. — H H, Mar 28 '12 at 09:13

score 40 · Accepted Answer · edited Feb 01 '19 at 16:07

40

You can divide your class up into exposed methods and private inner methods. The inner methods do the logic like fetching and the public methods perform the locking. Example:

class FooLocker 
{ 
    ReaderWriterLockSlim locker = new ReaderWriterLockSlim(); 
    List<Foo> fooList = new List<Foo>(); 


    public void ChangeFoo(int index, string bar) 
    { 
        locker.EnterWriteLock(); 

        try 
        { 
            Foo foo = UnsafeGetFoo(index); 
            foo.Bar = bar; 
        } 
        finally 
        { 
            locker.ExitWriteLock(); 
        } 
    } 

    public Foo GetFoo(int index)  
    { 
        locker.EnterReadLock();  

        try 
        { 
            return UnsafeGetFoo(index);
        } 
        finally 
        { 
            locker.ExitReadLock(); 
        } 
    } 

    private Foo UnsafeGetFoo(int index)
    {
        return fooList[index]; 
    }
}

edited Feb 01 '19 at 16:07

n.sh

347
4
19

answered Mar 28 '12 at 08:59

Polity

14,734
2
40
40

1

Great answer, thanks! Just have to make sure the private unsafe method has adequate documentation to make sure nobody uses it without ensuring locking is in place. – Adam Rodger Mar 28 '12 at 09:34
4

@AdamRodger document and do `System.Diagnostics.Debug.Assert(locker.IsReadLockHeld || locker.IsUpgradableReadLockHeld)` – Kasper van den Berg Aug 13 '15 at 14:32
1

I don't get what's the difference, you just moved the code in the try to a method, why does it matter? – shinzou Nov 11 '17 at 16:07
@shinzou because the recursion happens by a method that locks calling another method that locks. This separates those methods that enter the lock and those that require (and ideally, assert) that it has been entered, allowing this to be avoided. – Jon Hanna Feb 01 '19 at 16:19

C# ReaderWriterLockSlim Best Practice to Avoid Recursion

1 Answers1