Legitimate use cases for UpgradeableReadLock

Question

I was recently reminded of the UpgradeableReadLock construct C# provides and I'm trying to discern when it really makes sense to use it.

Say, for example, I have a cache of settings that are heavily read by many classes, but periodically need to be updated with a very low frequency based on a set of conditions that aren't necessarily deterministic...

would it make more sense to simply lock like so:

List<Setting> cachedSettings = this.GetCachedSettings( sessionId );

lock(cachedSettings)
{
    bool requiresRefresh = cachedSettings.RequiresUpdate();
    if(requiresRefresh)
    {
        // a potentially long operation
        UpdateSettings( cachedSettings, sessionId );
    }

    return cachedSettings;
}

or use an UpgradeableReadLock:

public class SomeRepitory {

private ReaderWriterLockSlim _rw = new ReaderWriterLockSlim();

public List<Setting> GetCachedSettings( string sessionId )
{
    _rw.EnterUpgradeableReadLock();

    List<Setting> cachedSettings = this.GetCachedSettings( sessionId );

    bool requiresRefresh = cachedSettings.RequiresUpdate();
    if(requiresRefresh)
    {
        _rw.EnterWriteLock();

        UpdateSettings( cachedSettings, sessionId );

        _rw.ExitWriteLock();
    }

    _rw.ExitUpgradeableReadLock();

    return cachedSettings;
}

perhaps what confuses me the most is how we can get away with checking if an update is required outside of the write block. In my example above I am referring to when I check for where a refresh is required, but to simplify I'll use an example from "C# 5.0 In A Nutshell":

while (true)
{
    int newNumber = GetRandNum (100); 
    _rw.EnterUpgradeableReadLock(); 
    if (!_items.Contains (newNumber)) 
    {
        _rw.EnterWriteLock();
        _items.Add (newNumber);
        _rw.ExitWriteLock();
        Console.WriteLine ("Thread " + threadID + " added " + newNumber);
    }
    _rw.ExitUpgradeableReadLock();
    Thread.Sleep (100);
}

my understanding is that this allows concurrent reads unless a thread needs to write, but what if two or more threads end up with the same random number and determine !_items.Contains(newNumber)? Given my understanding that this should allow concurrent reads (and correct me if I have misunderstood, of course).. it seems that, as soon as a write lock is obtained, any threads that were concurrently reading would need to be suspended and forced back to the start of _rw.EnterUpgradeableReadLock(); ?

Answer 1

Of course your second approach is better in case of many simultaneous readers and relatively rare write operations. When read lock is acquired (using _rw.EnterUpgradeableReadLock()) by a thread - other threads can also acquire it and read the value simultaneously. When some thread then enters write lock, it waits all reads to complete and then acquires exclusive access to lock object (all other threads trying to execute EnterXXX() operations wait) to update the value. When it releases the lock, other threads can do their job.

First example lock(cachedSettings) blocks all other threads so that only one thread can read the value at a time.

I would recommend in addition use the following pattern:

_rw.EnterUpgradeableReadLock();
try
{
    //Do your job
}
finally
{
    _rw.ExitUpgradeableReadLock();
}

for all Enter/Exit lock operations. It ensures (with high probability) that if exception happened inside your synchronized code, lock won't remain locked forever.

EDIT: Answering Martin's comment. If you don't want multiple threads updating the value simultaneously, you need to change your logic to achieve that. For example, using a double-checked lock construct:

if(cachedSettings.RequiresUpdate())
{
    _rw.EnterWriteLock();
    try
    {
        if(cachedSettings.RequiresUpdate())
        {
            UpdateSettings( cachedSettings, sessionId );
        }
    }
    finally
    {
        _rw.ExitWriteLock();
    }
}

This will check if while we were waiting for write lock other thread haven't refreshed that value already. And if value doesn't require refresh anymore - just release the lock.

IMPORTANT: it's very bad to take exclusive lock for long time. So it the UpdateSettings function is long-running, you better execute it outside the lock and implement some additional logic to allow readers read expired value while some thread is refreshing it. I used to implement cache once and it's really complex to make it fast and thread-safe. You better use one of the existing implementations (for example System.Runtime.MemoryCache).