HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Reader-writer collection

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
writercollectionreader

Problem

This is based on Alexey Drobyshevsky's excellent article, "Problems with Iteration". With Alexey's suggestion, I implemented his solution using a ReaderWriterLockSlim. Then I fleshed out a ThreadSafeList : IList implementation.

The idea here is to have a collection that multiple threads can iterate over, execute Linq queries against, and modify at the same time without data inconsistencies, InvalidOperationExceptions, or deadlocks.

```
using System.Collections;
using System.Collections.Generic;
using System.Threading;
namespace ThreadSafeEnumeration
{

public class SafeEnumerator : IEnumerator
{
private readonly ReaderWriterLockSlim _readerWriterLock;
private readonly IEnumerator _innerCollection;

public SafeEnumerator(IEnumerator innerCollection, ReaderWriterLockSlim readerWriterLock)
{
_innerCollection = innerCollection;
_readerWriterLock = readerWriterLock;
_readerWriterLock.EnterReadLock();
}

public void Dispose()
{
_readerWriterLock.ExitReadLock();
}

public bool MoveNext()
{
return _innerCollection.MoveNext();
}

public void Reset()
{
_innerCollection.Reset();
}

public T Current
{
get { return _innerCollection.Current; }
}

object IEnumerator.Current
{
get { return Current; }
}
}

public class ThreadSafeList : IList
{
private readonly ReaderWriterLockSlim _readerWriterLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
private readonly List _innerList = new List();

IEnumerator IEnumerable.GetEnumerator()
{
_readerWriterLock.EnterReadLock();
try
{
return new SafeEnumerator(_innerList.GetEnumerator(), _readerWriterLock);
}
finally
{
_readerWriterLock.ExitReadLock();
}
}

public IEnumerator GetEnumerator

Solution

I see that you're using the ReaderWriterLockSlim constructor with the SupportsRecursion LockRecursionPolicy.

Notice this line in the documentation for ReaderWriterLockSlim:


Regardless of recursion policy, a thread that initially entered read mode is not allowed to upgrade to upgradeable mode or write mode, because that pattern creates a strong probability of deadlocks.

However, I don't see anything in your code that makes me think that this is causing you a problem. In fact, calls to the EnterWriteLock method will throw a SynchronizationLockException if you have previously called EnterReadLock, so I'm sure you would have noticed that happening.

I see that there are multiple places in your code where you are doing this:

try
    {
        _readerWriterLock.EnterWriteLock();
        // ...
    }
    finally
    {
        _readerWriterLock.ExitWriteLock();
    }


You should keep the call to EnterWriteLock outside of the try block, because if it fails for some reason, you don't want to call ExitWriteLock. The same goes for calls to EnterReadLock. It should be like this:

_readerWriterLock.EnterWriteLock();
    try
    {
        // ...
    }
    finally
    {
        _readerWriterLock.ExitWriteLock();
    }

Code Snippets

try
    {
        _readerWriterLock.EnterWriteLock();
        // ...
    }
    finally
    {
        _readerWriterLock.ExitWriteLock();
    }
_readerWriterLock.EnterWriteLock();
    try
    {
        // ...
    }
    finally
    {
        _readerWriterLock.ExitWriteLock();
    }

Context

StackExchange Code Review Q#7276, answer score: 2

Revisions (0)

No revisions yet.