Best C# solution for multithreaded threadsafe read/write locking?

Question

What is the safest (and shortest) way do lock read/write access to static members in a multithreaded environment in C#?

Is it possible to do the threadsafe locking & unlocking on class level (so I don't keep repeating lock/unlock code every time static member access is needed)?

Edit: Sample code would be great :)

Edit: Should I use the volatile keyword or Thread.MemoryBarrier() to avoid multiprocessor caching or is that unnecessary? According to Jon Skeet only those will make changes visible to other processors? (Asked this separately here).

Solution 1

The safest and shortest way is to create a private, static field of type Object that is only used for locking (think of it as a "pad-lock" object). Use this and only this field to lock on as this prevent other types from locking up your code when then lock on the same type that you do.

If you lock on the type itself there is risk that another type will also decide to lock on your type and this could create deadlocks.

Here is an example:

class Test
{
    static readonly Object fooLock = new Object();
    static String foo;

    public static String Foo
    {
        get { return foo; }
        set
        {
            lock (fooLock)
            {
                foo = value;
            }
        }
    }
}

Notice that I have create a private, static field for locking foo - I use that field to lock the write operations on that field.

Solution 2

Small Values

For small values (basically any field that can be declared volatile), you can do the following:

private static volatile int backingField;

public static int Field
{
    get { return backingField; }
    set { backingField = value; }
} 

Large Values

With large values the assignment won't be atomic if the value is larger then 32-bits on a 32-bit machine or 64-bits on a 64-bit machine. See the ECMA 335 12.6.6 spec. So for reference types and most of the built-in value types the assignment is atomic, however if you have some large struct, like:

struct BigStruct 
{
    public long value1, valuea0a, valuea0b, valuea0c, valuea0d, valuea0e;
    public long value2, valuea0f, valuea0g, valuea0h, valuea0i, valuea0j;
    public long value3;
}

In this case you will need some kind of locking around the get accessor. You could use ReaderWriterLockSlim for this which I've demonstrated below. Joe Duffy has advice on using ReaderWriterLockSlim vs ReaderWriterLock:

    private static BigStruct notSafeField;
    private static readonly ReaderWriterLockSlim slimLock = 
        new ReaderWriterLockSlim();

    public static BigStruct Safe
    {
        get
        {
            slimLock.EnterReadLock();
            var returnValue = notSafeField;
            slimLock.ExitReadLock();

            return returnValue;
        }
        set
        {
            slimLock.EnterWriteLock();
            notSafeField = value;
            slimLock.ExitWriteLock();
        }
    }

---

Unsafe Get-Accessor Demonstration

Here's the code I used to show the lack of atomicity when not using a lock in the get-accessor:

    private static readonly object mutexLock = new object();
    private static BigStruct notSafeField;

    public static BigStruct NotSafe
    {
        get
        {
            // this operation is not atomic and not safe
            return notSafeField;
        }
        set
        {
            lock (mutexLock)
            {
                notSafeField = value;
            }
        }
    }

    public static void Main(string[] args)
    {
        var t = new Thread(() =>
            {
                while (true)
                {
                    var current = NotSafe;
                    if (current.value2 != (current.value1 * 2)
                        || current.value3 != (current.value1 * 5))
                    {
                        throw new Exception(String.Format("{0},{1},{2}", current.value1, current.value2, current.value3));
                    }
                }
            });
        t.Start();
        for(int i=0; i<50; ++i)
        {
            var w = new Thread((state) =>
                {
                    while(true)
                    {
                        var index = (int) state;
                        var newvalue = new BigStruct();
                        newvalue.value1 = index;
                        newvalue.value2 = index * 2;
                        newvalue.value3 = index * 5;
                        NotSafe = newvalue;
                    }
                });
            w.Start(i);
        }
        Console.ReadLine();
    }

Solution 3

Although you could just use a single mutex to control all the access to the class (effectively serializing the access to the class) I suggest you study the static class, determine which members are being used where and how and the use one or several ReaderWriterLock (code examples in the MSDN documentation) which provides access to several readers but only one writer at the same time.

That way you'll have a fine grained multithreaded class which will only block for writing but will allow several readers at the same time and which will allow writing to one member while reading another unrelated member.

Solution 4

class LockExample {
    static object lockObject = new object();
    static int _backingField = 17;

    public static void NeedsLocking() {
        lock(lockObject) {
            // threadsafe now
        }
    }

    public static int ReadWritePropertyThatNeedsLocking {
        get {
            lock(lockObject) {
                // threadsafe now
                return _backingField;
            }
        }
        set {
            lock(lockObject) {
                // threadsafe now
                _backingField = value;
            }
        }
    }
}

lock on an object specifically created for this purpose rather than on typeof(LockExample) to prevent deadlock situations where others have locked on LockExample's type object.

Is it possible to do the threadsafe locking & unlocking on class level (so I don't keep repeating lock/unlock code every time static member access is needed)?

Only lock where you need it, and do it inside the callee rather than requiring the caller to do the locking.

Solution 5

Several others have already explained how to use the lock keyword with a private lock object, so I will just add this:

Be aware that even if you lock inside each method in your type, calling more than one method in a sequence can not be considered atomic. For example if you're implementing a dictionary and your interface has a Contains method and an Add method, calling Contains followed by Add will not be atomic. Someone could modify the dictionary between the calls to Contains and Add - i.e. there's a race condition. To work around this you would have to change the interface and offer a method like AddIfNotPresent (or similar) which encapsulates both the checking and the modification as a single action.

Jared Par has an excellent blog post on the topic (be sure to read the comments as well).

View more solutions

Comments

[Alex]

Do I need to use the volatile keywords to avoid multiprocessor caching of the variable or can I disregard this?

[Henk Holterman]

You need volatile (and a lot of taking care) if you don't lock.

[Joseph Kingry]

There needs to be a get lock if you are dealing with value types that are larger then 32-bits.

[Pop Catalin]

The lock in this example is useless, won't prevent any race conditions ... The correct way is not to syncronize at all at class level, but provide a SyncRoot object that all threads should lock on before doing a read write operation on the property.

[jrista]

I have to second Pop Catalin. Locking shouldn't be done internally...but support for locking should be exposed so that when it is needed, it can be achieved.

[Joseph Kingry]

As stated in comments to Andrew Hare's answer, the lock here isn't doing anything given an int-valued field.

[Oliver]

Watch out for the volatile keyword: as mentioned in this detailed article on threading a write followed by a read operation on a volatile field can still be reordered thus leading to very subtle, hard-to-find bugs.

[Falaque]

@Jason, Your code doesn't allow more than 1 thread to read the value. What is the problem in multiple thread reading the value?

[hross]

Agreed. I would not use this answer in any code I needed read/write locking. @JosephKingry has a better answer.

[atlaste]

P.S.: using try/finally is a good idea for these kinds of locks. You don't want to end up with an exception that's thrown that keeps the thing locked.