views:

405

answers:

3

Hi,

I need to make an existing app thread safe. Due circumstances (see below), I decided to use one single ReaderWriterLock for the entire graph of business objects. All methods/properties must look like these:

public int MyReadOperation(string inputParam)
{
   rwLock.AcquireReaderLock(10000);
   try
   {
      // do all read operations
      ...
   }
   finally
   {
      rwLock.ReleaseReaderLock();
   }
}

public void MyWriteOperation(string input)
{
   rwLock.AcquireWriterLock(10000);
   try
   {
      // do all write operations
      ...
   }
   finally
   {
      rwLock.ReleaseWriterLock();
   }

}

But I have immense amount of methods to cover and I am freaked out from the idea of copy/pasting. Inspired by MethodImplAttribute, I would prefer to have a code like this while behave as the code above:

[ReadOperation]
public int MyReadOperation(string inputParam)
{
   // do all read operations
   ...
}

[WriteOperation]    
public void MyWriteOperation(string input)
{
   // do all write operations
   ...
}

Is there a way to interrupt Thread execution before/after entering into a property or a method and adding the thread-safety precautions? Or somehow utilizing the functional language features of C#, embedding the productive body of the methods into a generic ReaderWriterLock aquiring "frame"?

A bit of background:

I am working on a project where data carrier business objects are exposed via .NET Remoting. However, these data classes are not serializable but MarshalByRef-s. That means that ALL clients actually read/write the very same business objects. This cannot be changed, it is carved in stone. The hope for thread-safety is that these remoted business objects are read-only in the eyes of the remoting clients (thought they do loop many lists) and all write operations are nicely separated into a dedicated facade. I expect rare writes and frequent reads. The business objects are highly connected, they are very "graphy".

+7  A: 

I would do something like that using PostSharp.

It runs as an extra build step and inserts the corresponding MSIL, but it will do what you want. The PostSharp attribute definitions would look something like this (depending on your exact implementation). These could then be used as you describe above.

public sealed class ReadOperationAttribute : OnMethodBoundaryAspect
{
  // not quite sure how you manage your lock, so put this dummy method in.
  ReaderWriterLock _rwLock = ReaderWriterLock.GetCorrectReaderWriterLock();

  [ThreadStatic]
  static bool _isLocked;

  public override void OnEntry( MethodExecutionEventArgs e )
  {
    try
    {
        _rwLock.AcquireReaderLock(10000);
        _isLocked = true;
    }
    catch
    {
        _isLocked = false;
        throw;
    }
  } 

  public override void OnExit( MethodExecutionEventArgs e )
  {    
      if (_isLocked) 
      {
          _rwLock.ReleaseReaderLock();
          _isLocked = false;
      }
  } 
}

public sealed class WriteOperationAttribute : OnMethodBoundaryAspect
{
  // not quite sure how you manage your lock, so put this dummy method in.
  ReaderWriterLock _rwLock = ReaderWriterLock.GetCorrectReaderWriterLock();

  [ThreadStatic]
  static bool _isLocked;

  public override void OnEntry( MethodExecutionEventArgs e )
  {
     try
    {
        _rwLock.AcquireWriterLock(10000);
        _isLocked = true;
    }
    catch
    {
        _isLocked = false;
        throw;
    }
  } 

  public override void OnExit( MethodExecutionEventArgs e )
  {
      if (_isLocked) 
      {
          _rwLock.ReleaseReaderLock();
          _isLocked = false;
      }
  } 
}

EDIT: Updated to address concerns. (Untested ;) ) Also, note as I said in the comment to the question, Microsoft recommends using ReaderWriterLockSlim in preference to ReaderWriterLock.

Andrew Rollings
exactly what i would do. :) +1
Frederik Gheysels
Why, thank you :)
Andrew Rollings
While this is my favorite answer, it is not good enough, due to a small but critical detail. The override of OnEntry causes the acquiring of the lock going INSIDE the "try" section. Should the acquiring of the lock fail (and throw an exception), the "finally" section will still run.
Mr. Lame
Well, that would be easy enough to keep track of, assuming the lock has an 'IsReadLockedOnThisThread' type member... If not, use a boolean flag.
Andrew Rollings
Make that a ThreadLocal boolean flag ;)
Andrew Rollings
In the CATCH section after setting _isLocked, you should re-throw the exception.
Mr. Lame
+1  A: 

Using a post-build step might be a good solution. In pure code, I would create extension methods on ReaderWriterLock like this:

public static void ReadLock(this ReaderWriterLock l, Action f) {
  l.AquireReaderLock();
  try { f(); }
  finally { l.ReleaseReaderLock(); }
}

And one for write. If the lock is truely global, you could even make static methods. Then your code looks a lot less annoying:

public void DoSomething() { theLock.ReadLock( () => { // Existing code }); }

This has the benefit of being fine grained, so parts of a method that don't need sync won't be sync'd. But, it still does require making changes to each method. Fortunately they're relatively small, and all the locking code you'd end up changing is at least in one place.

MichaelGG
+3  A: 

First of all, thanx to Andrew pointing me to PostSharp. Based on his answer, I ended up with this final one.

[Serializable]
public class ReadOperationAttribute : PostSharp.Laos.OnMethodInvocationAspect
{
 public override void  OnInvocation(MethodInvocationEventArgs eventArgs)
 {
        ReaderWriterLock rwLock = GetLock();
  rwLock.AcquireReaderLock(10000);
  try { eventArgs.Proceed(); }
  finally { rwLock.ReleaseReaderLock();}
 }
}
public class Foo
{
 [ReadOperation]
 public string Bar
 {

  get { return "stuff"; }
  set { Console.WriteLine(value); }
 }

 [ReadOperation]
 public void Test(string input)
 {
  Console.WriteLine(input);
 }
}

It does exactly what I expressed in the question and is perfectly debuggable. We can make it even more useful, if we declare the attribute for an entire assembly:

[assembly: ReadOperation(AttributeTargetElements=MulticastTargets.Method,
AttributeTargetTypes="MyNamespace.*")]

This way we do not have to decorate every method/property, but placing this attribute in the assembly once, we braket all our methods/properties with ReaderWriterLock.

Also, as it has been pointed out by Andrew, Microsoft recommends to use ReaderWriterLockSlim if you use .NET3.5 or above.

Mr. Lame
You should have accepted the actual answer that helped you and not your own.
dimitrisp
No! I should accept the proper answer to help other readers! I did give the proper credit to the Andrew for his very valuable contribution. (Btw, by accepting my own answer, I do not get reward points)
Mr. Lame
Meh. I'm not in it for the points :)
Andrew Rollings