Monday, April 02, 2007

I was just talking to J.D. Conley (part of the Coversant team. He also has a blog on the net) about some if the issues i've been having in monotorrent with threading and async sockets. He had a few good ideas about what i could try, so hopefully over the next week or three i'll be able to test out a few of the ideas to help improve performance. Anyway, as part of the discussion i realised that i could probably replace a lot of my lock(object) statements with a much nicer ReaderWriterLock().

The way the code works in monotorrent is that i read from my lists of peers much more than i modify them, therefore it makes sense to have a locking structure that allows multiple readers and a single writer! This gave me the idea for this blog post.

You see, the problem with the ReaderWriter lock is that you have to be careful in how you use it. It's not as simple as the lock(object) statement. If you forget to release a reader lock, or worse, a writer lock, you will experience deadlocking and it will be very hard to track down the exact place where you haven't released the lock!

Also, suppose you want to a writer lock, but you forget to check if your thread already has a reader lock, you will end up throwing an exception as you will not be able to get the writer lock.

However, it's very easy to abstract away all those horrible problems with two simple structs:

public struct ReaderLock : IDisposable
{
public ReaderWriterLock Locker;

public ReaderLock(ReaderWriterLock locker)
{
Locker = locker;
locker.AcquireReaderLock(1000);
}

public void Dispose()
{
Locker.ReleaseReaderLock();
}
}

public struct WriterLock : IDisposable
{
private bool upgraded;
private LockCookie cookie;
public ReaderWriterLock Locker;

public WriterLock(ReaderWriterLock locker)
{
Locker = locker;
upgraded = locker.IsReaderLockHeld;
cookie = default(LockCookie);

if (upgraded)
cookie = locker.UpgradeToWriterLock(1000);
else
locker.AcquireWriterLock(1000);
}

public void Dispose()
{
if (upgraded)
Locker.DowngradeFromWriterLock(ref cookie);
else
Locker.ReleaseWriterLock();
}
}

Suppose you have the following code snippet:
ReaderWriterLock myLocker = new ReaderWriterLock();

Instead of having to manually use: myLocker.AcquireReaderLock() and remember to call myLocker.AcquireWriterLock() in a finally statement, and instead of messing around trying to decide if you should call myLocker.AcquireWriterLock or myLocker.UpgradeReaderLock() you can just use the corresponding ReaderLock or WriterLock as needed inside a using statement!

For example, take this code snippet:

try
{
DoSomeStuffWhichMayOrMayNotThrowAnException();
myLocker.AcquireReaderLock();
DoLotsOfStuff();
myLocker.ReleaseReaderLock();
}
catch(WhateverException)
{
if(myLocker.IsReaderLockHeld)
myLocker.ReleaseReaderLock();

DoRestOfErrorHandling();
}

You can use the much nicer:

try
{
DoSomeStuffWhichMayOrMayNotThrowAnException();
using(new ReaderLock(myLocker)
DoLotsOfStuff();
}
catch(WhateverException)
{
DoRestOfErrorHandling();
}

Supposing you actually needed a writer lock, just change the using statement to:
using(new WriterLock(myLocker)

The constructor for writer lock will then check to see if it should upgrade an existing reader or if it should get a standard WriterLock. If it upgraded, it will remember to downgrade it correctly later.

Using this method not only makes you need less lines of code, makes your code more readable, but it also completely removes the possibility of creating a deadlock by accidentally forgetting to release a lock!

4 comments:

duff said...

just a little thing, use finally to release locker is better than done it in code and in catch.

knocte said...

This code snippet is wonderful!

BTW, another interesting lock is one that has priorities in the waiting queue, like the PriorityLock.

It could be nice to have the best of two worlds, for example, a PriorityFifoReaderWriterLock (Fifo so as to have a first-in-first-serve queue for threads which request the same priority, which BTW is not the algorithm used by the lock statement nor the semaphore).

Alan said...

@Duff: You're right, it would be much better to use additional try/finally blocks to correctly handle the release of the lock, but the point i was trying to make was that it is *much* easier to use the helper struct.

A single using(new WriterLock(blah)) or using(new ReaderLock(blah)) statement guarantees perfect usage of the ReaderWriterLock class in all circumstances.

It also reduces your lines of code, which is a plus, and also makes it blantantly obvious *exactly* where the lock will be obtained and removed ;)

rektide said...

I like the extension methods rendition where you add a property to ReaderWriterLock that returns the objects for you, instead of doing new ReaderLock(this._rwLock).

using(_rwLock.newReaderLock) {}
v.
using(new ReaderLock(_rwLock) {}

just a different spice, same stuff.

Hit Counter