RTFM–the tale of ReaderWriterLockSlim

No comments

Tuesday, June 28, 2011

The .NET BCL (Base Class Library) is readable, easy to understand and not less important documented. This is a story on where a simple reading of documentation could have saved us from a minor bug…

If you’ve been using .NET you might be familiar with the ReaderWriterLock essentially a lock that enable multiple readers or one writer synchronization. The only problem is that the good old ReaderWriterLock had a few drawbacks – and so the good people of Microsoft have create a new and improved ReaderWriterLockSlim.

A developer at my team had a simple task – make sure that there are not race conditions when reading and writing a big chunk of the project’s shared data and for the task he choose the trusty ReaderWriterLockSlim. He had two methods to access the data one for reading and one for writing – so far so good.

Fast forward a year and the team has a strange issue – it seems that only one thread can access the data – even if it’s only reading it. After a few debates my co-worker found the root of the problem – whenever a read was required instead of using EnterReadLock – which is used to, well acquire read permissions the original developer had used EnterUpgradeableReadLock. Perhaps because it’s enable using a write request from within the read lock – I have no idea since that developer has since left the team.

And according to MSDN:

Regardless of recursion policy, only one thread can be in write mode at any time. When a thread is in write mode, no other thread can enter the lock in any mode. Only one thread can be in upgradeable mode at any time. Any number of threads can be in read mode, and there can be one thread in upgradeable mode while other threads are in read mode.


The problem was not that the original developer didn’t know that using an upgradable lock essentially creates a state where all the locks a mutually exclusive – just like using simple lock only without the performance benefits. The problem is that all of the team saw the code at one point or the other and not one of use thought of reading the page on MSDN that has the paragraph quoted above!

We had a simple policy of if it works – don’t break it and would have probably continue to use ReaderWriterLockSlim the wrong way unless a different issue caused us to take notice.


Happy coding…

A co-worker showed me this:

var result = collection1.GetObjectByAtSpecialLocation(location) ??
        collection1.GetObjectByAtLocation(parent, location) ??
        collection2.GetObjectByAtSpecialLocation(location) ??
        collection2.GetObjectByAtLocation(parent, location);

I’m still trying to decide whether it’s a good practice and if it’s readable or not…

In case you’re not familiar with ?? operator a.k.a null-coalescing operator it’s basically returns a default value if the left side is null so the following line means return empty string if name is null:

string result = name ?? string.Empty;

And so the code at the beginning of the post can also be written as:

var result = collection1.GetObjectByAtSpecialLocation(location);
if(result == null) 
    result = collection1.GetObjectByAtLocation(parent, location);

if(result == null) 
    result = collection2.GetObjectByAtSpecialLocation(location);

if(result == null) 
    result = collection2.GetObjectByAtLocation(parent, location);

Although most developers would agree that it’s easy enough to understand it’s far from being a good piece of code.

I also think that the reason that there was a need to check if an item exist over two different collections in two different ways indicates a design flow – but since we’re not living in a perfect world right now it’s impossible to merge those two collections and regardless of effort there was a good reason that they are different in the first place so I’m have to find an item in four different ways.

At first when I saw the code I cried abuse – it seemed to me that there bound to be a better way to do so without nesting four calls but after a while this solution seemed almost elegant.

What do you think?

Related Posts Plugin for WordPress, Blogger...