CODEWITHSUNDEEP
c#thread-safetyreaderwriterlockslim
Igor Pashchuk
Igor Pashchuk

Reputation: 2501

Does this ReaderWriterLockSlim locking pattern look right?

I've been using ReaderWriterLockSlim to synchronize reads/writes and it has worked quite well so far.

I have a collection of objects and read/write locks are maintained in a Dictionary<string, ReaderWriterLockSlim>. I have a scenario where I need to acquire multiple read locks atomically. I'm certain I don't have any code that would allow one thread to attempt to hold more than one write lock concurrently, which makes me think that the code below will work without problems.

If I were trying the approach below with write locks instead of read locks, I'm almost certain I would end up with deadlocks unless I can ensure that locking always happens in the same order (which I can't in my case).

Does anyone see any problems with this code assuming the following:

  1. Elsewhere, no thread holding a write lock would be allowed to hold any other lock (read or write)
  2. Multiple threads could hold any number of read locks concurrently

Any thoughts?

public void WithReaderLocksForItemsNamed(string[] itemNames, Action action)
    {
        // this gets locks for the items specified from my internation dictionary
        ReaderWriterLockSlim[ ] locks = LocksForItemsNamed( itemNames ) ;

        try
        {
            foreach( var @lock in locks )
            {
                @lock.EnterReadLock( ) ;
            }

            action( ) ;
        }
        finally
        {
            foreach( var @lock in locks )
            {
                if (@lock.IsReadLockHeld)
                {
                    @lock.ExitReadLock( ) ;
                }
            }
        }
    }

Upvotes: 1

Views: 539

Answers (2)

Igor Pashchuk
Igor Pashchuk

Reputation: 2501

This is an old question but I thought it would still be good to update it with the final solution. Although I never experienced problems with the above, I applied the prudent practice of exiting locks in the reverse order they were acquired. Thus the final code looks something like this:

public void WithReaderLocksForItemsNamed(string[] itemNames, Action action)
{
    // this gets locks for the items specified from my internation dictionary
    ReaderWriterLockSlim[ ] locks = LocksForItemsNamed( itemNames ) ;

    try
    {
        foreach( var @lock in locks )
        {
            @lock.EnterReadLock( ) ;
        }

        action( ) ;
    }
    finally
    {
        foreach( var @lock in locks.Reverse() )
        {
            if (@lock.IsReadLockHeld)
            {
                @lock.ExitReadLock( ) ;
            }
        }
    }
}

Upvotes: 0

Steve Dunn
Steve Dunn

Reputation: 21791

It looks OK but you might need a re-entrancy lock on the method itself and of course, one where you iterate your source list to get the locks.

Upvotes: 1

Related Questions