Is This a Good Design for Creating Thread-Safe Classes in C#?

Question

Often, when I want a class which is thread-safe, I do something like the following:

public class ThreadSafeClass
{
    private readonly object theLock = new object();

    private double propertyA;
    public double PropertyA
    {
        get
        {
            lock (theLock)
            {
                return propertyA;
            }
        }
        set
        {
            lock (theLock)
            {
                propertyA = value;
            }
        }
    }

    private double propertyB;
    public double PropertyB
    {
        get
        {
            lock (theLock)
            {
                return propertyB;
            }
        }
        set
        {
            lock (theLock)
            {
                propertyB = value;
            }
        }
    }

    public void SomeMethod()
    {
        lock (theLock)
        {
            PropertyA = 2.0 * PropertyB;
        }
    }
}

It works, but it is very verbose. Sometimes I even create a lock object for each method and property creating more verbosity and complexity.

I know that it is also possible to lock classes using the Synchronization attribute but I'm not sure how well that scales -- as I often expect to have hundreds of thousands, if not millions, of instances of thread-safe objects. This approach would create a synchronization context for every instance of the class, and requires the class to be derived from ContextBoundObject and therefore could not be derived from anything else -- since C# doesn't allow for multiple inheritance -- which is a show stopper in many cases.

Edit: As several of the responders have emphasized, there is no "silver bullet" thread-safe class design. I'm just trying to understand if the pattern I'm using is one of the good solutions. Of course the best solution in any particular situation is problem dependent. Several of the answers below contain alternative designs which should be considered.

Edit: Moreover, there is more than one definition of thread safety. For example, in my implementation above, the following code would NOT be thread-safe:

var myObject = new ThreadSafeClass();
myObject.PropertyA++; // NOT thread-safe

So, does the class definition above represent a good approach? If not, what would you recommend for a design with similar behavior which would be thread-safe for a similar set of uses?

Answer 1

Bear in mind that the term "thread safe" is not specific; what you're doing here would be more accurately referred to as "synchronization" through the use of a Monitor lock.

That said, the verbosity around synchronized code is pretty much unavoidable. You could cut down on some of the whitespace in your example by turning things like this:

lock (theLock)
{
    propertyB = value;
}

into this:

lock (theLock) propertyB = value;

As to whether or not this is the right approach for you we really need more information. Synchronization is just one approach to "thread safety"; immutable objects, semaphores, etc. are all different mechanisms that fit different use-cases. For the simple example you provide (where it looks like you're trying to ensure the atomicity of a get or set operation), then it looks like you've done the right things, but if your code is intended to be more of an illustration than an example then things may not be as simple.

Answer 2

There is no "one-size-fits-all" solution to the multi-threading problem. Do some research on creating immutable classes and learn about the different synchronization primitives.

This is an example of a semi-immutable or the-programmers-immutable class .

public class ThreadSafeClass
{
    public double A { get; private set; }
    public double B { get; private set; }
    public double C { get; private set; }

    public ThreadSafeClass(double a, double b, double c)
    {
        A = a;
        B = b;
        C = c;
    }

    public ThreadSafeClass RecalculateA()
    {
        return new ThreadSafeClass(2.0 * B, B, C);
    }
}

This example moves your synchronization code into another class and serializes access to an instance. In reality, you don't really want more than one thread operating on an object at any given time.

public class ThreadSafeClass
{
    public double PropertyA { get; set; }
    public double PropertyB { get; set; }
    public double PropertyC { get; set; }

    private ThreadSafeClass()
    {

    }

    public void ModifyClass()
    {
        // do stuff
    }

    public class Synchronizer
    {
        private ThreadSafeClass instance = new ThreadSafeClass();
        private readonly object locker = new object();

        public void Execute(Action<ThreadSafeClass> action)
        {
            lock (locker)
            {
                action(instance);
            }
        }

        public T Execute<T>(Func<ThreadSafeClass, T> func)
        {
            lock (locker)
            {
                return func(instance);
            }
        }
    }
}

Here is a quick example of how you would use it. It may seem a little clunky but it allows you to execute many actions on the instance in one go.

var syn = new ThreadSafeClass.Synchronizer();

syn.Execute(inst => { 
    inst.PropertyA = 2.0;
    inst.PropertyB = 2.0;
    inst.PropertyC = 2.0;
});

var a = syn.Execute<double>(inst => {
    return inst.PropertyA + inst.PropertyB;
});

Answer 3

As per my comment above - it gets a little hairier if you want simultaneous readers allowed but only one writer allowed. Note, if you have .NET 3.5, use ReaderWriterLockSlim rather than ReaderWriterLock for this type of pattern.

public class ThreadSafeClass
{
    private readonly ReaderWriterLock theLock = new ReaderWriterLock();

    private double propertyA;
    public double PropertyA
    {
        get
        {
            theLock.AcquireReaderLock(Timeout.Infinite);
            try
            {
                return propertyA;
            }
            finally
            {
                theLock.ReleaseReaderLock();
            }
        }
        set
        {
            theLock.AcquireWriterLock(Timeout.Infinite);
            try
            {
                propertyA = value;
            }
            finally
            {
                theLock.ReleaseWriterLock();
            }
        }
    }

    private double propertyB;
    public double PropertyB
    {
        get
        {
            theLock.AcquireReaderLock(Timeout.Infinite);
            try
            {
                return propertyB;
            }
            finally
            {
                theLock.ReleaseReaderLock();
            }
        }
        set
        {
            theLock.AcquireWriterLock(Timeout.Infinite);
            try
            {
                propertyB = value;
            }
            finally
            {
                theLock.ReleaseWriterLock();
            }
        }
    }

    public void SomeMethod()
    {
        theLock.AcquireWriterLock(Timeout.Infinite);
        try
        {
            theLock.AcquireReaderLock(Timeout.Infinite);
            try
            {
                PropertyA = 2.0 * PropertyB;
            }
            finally
            {
                theLock.ReleaseReaderLock();
            }
        }
        finally
        {
            theLock.ReleaseWriterLock();
        }
    }
}

Answer 4

You may find the Interlocked class helpful. It contains several atomic operations.

Answer 5

One thing you could do that could help you avoid the extra code is use something like PostSharp to automatically inject those lock statements into your code, even if you had hundreds of them. All you'd need is one attribute attached to the class, and the attribute's implementation which would add the extra locking variables.

Answer 6

I know this might sound like an smart a** answer but ... the BEST way to develop threadsafe classes is to actually know about multithreading, about its implications, its intricacies and what does it implies. There's no silver bullet.

• First you need a good reason to use it. Threads are a tool, you don't want to hit everything with your new found hammer.
• Secondly, learn about the problems of multithreading... deadlocks, race conditions, starvation and so on
• Third, make sure is worth it. I'm talking about benefit/cost.
• Finally... be prepared to heavy debugging. Debugging multithreaded code is much more difficult than standard old sequential code. Learn some techniques about how to do that.

Seriously... don't try to multithread (in production scenarios I mean) until you know what you're getting yourself into... It can be a huge mistake.

Edit: You should of course know the synchronization primitives of both the operating system and your language of choice (C# under Windows in this case, I guess).

I'm sorry I'm not giving just the code to just make a class threadsafe. That's because it does not exist. A completely threadsafe class will probably just be slower than just avoiding threads and will probably act as a bottleneck to whatever you're doing... effectively undoing whatever you thing you're achieving by using threads.

Answer 7

Since no else seems to be doing it, here is some analysis on your specific design.

• Want to read any single property? Threadsafe
• Want to update to any single property? Threadsafe
• Want to read a single property and then update it based on its original value? Not Threadsafe

Thread 2 could update the value between thread 1's read and update.

• Want to update two related properties at the same time? Not Threadsafe

You could end up with Property A having thread 1's value and Property B having thread 2's value.

1. Thread 1 Update A
2. Thread 2 Update A
3. Thread 1 Update B

4. Thread 2 Update B

   • Want to read two related properties at the same time? Not Threadsafe

Again, you could be interrupted between the first and second read.

I could continue, but you get the idea. Threadsafety is purely based on how you plan to access the objects and what promises you need to make.