Using the C# Volatile keyword in Threaded Application

Question

I have a class that has a few arraylists in it.

My main class creates a new instance of this class. My main class has at least 2 threads adding and removing from my class with the arraylists in it. At the moment everything is running fine but I was just wondering if it would be safer to declare my class with the arraylists in it as volatile eg/

private volatile myclass;
myclass = new myclass();
......
myclass.Add(...)
myclass.Clear(..)

Answer 1

At present, the code is wrong; adding a volatile keyword won't fix it. It's not safe to use the .NET classes across threads without adding synchronisation.

It's hard to give straightforward advice without knowing more about the structure of your code. A first step would be to start using the lock keyword around all accesses to the list object; however, there could still be assumptions in the code that don't work across multiple threads.

It's possible to use a collection class that's already safe for multithreaded access, which would avoid the need for getting the lock keyword in the right place, but it's still possible to make errors.

Can you post some more of your code? That way we can give more specific suggestions about making it thread safe.

Answer 2

Using the volatile keyword will not make your code thread-safe in this example. The volatile keyword is typically used to ensure that when reading or writing the value of a variable (i.e. class field) that the latest value for that variable is either read from main memory or written straight to main memory, rather than read from cache (e.g. a CPU register) for example. The volatile keyword is a way of saying "do not use caching optimizations with this shared field", and removes the issue where threads may use local copies of a field and so not see each other's updates.

In your case the value of myclass is not actually being updated (i.e. you are not re-assigning myclass) so volatile is not useful for you, and it is not the update of the myclass variable you actually want to make thread-safe in this case anyway.

If you wish to make updating of the actual class thread-safe, then using a "lock" around "Add" and "Clear" is a straight-forward alternative. This will ensure that only one thread at a time can do these operations (which update the internal state of myclass) and so should not be done in parallel.

A lock can be used as follows:

private readonly object syncObj = new object(); 
private readonly myclass = new myclass();
......

lock (syncObj)
{
    myclass.Add(...)
}

lock (syncObj)
{
    myclass.Clear(..)
}

You also need to add locking around any code that reads the state that is being updated by "Add", if that is the case although it does not appear in your example code.

It may not be obvious when first writing multi-threaded code why you would need a lock when adding to a collection. If we take List or ArrayList as an example, then the problem arises as internally these collections use an Array as a backing store, and will dynamically "grow" this Array (i.e. by creating a new larger Array and copying the old contents) as certain capacities are met when Add is called. This all happens internally and requires the maintenance of this Array and variables such as what current size the collection is (rather than the Length of the actual array which might be larger). So Adding to the collection may involve multiple steps if the internal Array needs to grow. When using multiple threads in an unsafe manner, multiple threads may indirectly cause growing to happen when Adding, and thus trample all over each others updates. As well as the issue of multiple threads Adding at the same time, there is also the issue that another thread may be trying to read the collection whilst the internal state is being changed. Using locks ensures that operations like these are done without interference from other threads.