Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

In current project I'm using a lot ReaderWriterLockSlim for synchronizing reading and writing values. Handling try { EnterReadLock(); .... } finaly { ExitReadLock(); } every time I need access value seems to me as copying code. So I created extension methods for ReaderWriterLockSlim which makes me coding much easer and faster (and reusable i think).

However I would like know, if you found some disadvantages in the extension methods I missed or forgot. There are three extension methods:

  • ReadOnly<T>(this ReaderWriterLockSlim self, Func<T> readFunc) // calls EnterReadLock()
  • Read<T>(this ReaderWriterLockSlim self, Func<T> readFunc) // calls EnterUpgradeableReadLock()
  • Write(this ReaderWriterLockSlim self, Action writeAction) // calls EnterWriteLock()
  • Write<T>(this ReaderWriterLockSlim self, Func<T> writeAction) // calls EnterWriteLock()

There is code of ReadOnly<T>(...) method. All methods looks similar.

public static T ReadOnly<T>(this ReaderWriterLockSlim self, Func<T> readFunc) {
    if (object.ReferenceEquals(self, null)) { throw new ArgumentNullException("self"); }
    if (object.ReferenceEquals(readFunc, null)) { throw new ArgumentNullException("readFunc"); }

    T result;

    // flag, if lock was entered
    bool lockEntered = false;
    try {
        // I don't want the ThreadAbortException during claiming the lock
        Thread.BeginCriticalRegion();
        try {
            self.EnterReadLock();
            lockEntered = true;
        }
        finally {
            Thread.EndCriticalRegion();
        }

        result = readFunc();
    }
    finally {
        // I don't want the ThreadAbortException during releasing the lock
        Thread.BeginCriticalRegion();
        try {
            if (lockEntered) {
                self.ExitReadLock();
            }
        }
        finally {
            Thread.EndCriticalRegion();
        }
    }

    return result;
}
share|improve this question
    
In cases like this, I prefer methods that work with using, instead of taking a delegate. –  svick Nov 28 '12 at 17:29
    
@svick, but working with using is not fully thread-safe :( –  TcKs Nov 29 '12 at 0:17
    
What do you mean? I don't see any reason why it shouldn't be thread-safe (assuming you don't use it incorrectly). –  svick Nov 29 '12 at 0:31
    
@svick: its because the using(var x = SomeFunc()){...} is only syntax sugar, when ThreadAbortException is thrown before object is passed from return value of "SomeFunc()" to local variable "x", the "finally{...}" block will not handle passed object, because in "x" is nothing (null). –  TcKs Dec 14 '12 at 18:02

1 Answer 1

It looks pretty good. I can't see anything less than great.
I'd just add that to your code:

    private static void Critical(Action criticalAction)
    {
        try
        {
            Thread.BeginCriticalRegion();
            criticalAction();
        }
        finally
        {
            Thread.EndCriticalRegion();
        }
    }

And i used the IsWriteLockHeld, IsReadLockHeld, etc, before exiting the lock.

share|improve this answer
    
Good point. Didn't you mean the "Thread.BeginCriticalRegion()" should be before "criticalAction()". –  TcKs Dec 14 '12 at 18:03
    
You got a point, there! –  NoProblemBabe Dec 14 '12 at 18:49

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.