C# multithreaded list operations

Question

If I have something like this (pseudocode):

class A
{
    List<SomeClass> list;

    private void clearList()
    {
        list = new List<SomeClass>();
    }

    private void addElement()
    {
        list.Add(new SomeClass(...));
    }
}

is it possible that I run into multithreading problems (or any kind of unexpected behavior) when both functions are executed in parallel?

The use case is a list of errors, which could be cleared at any time (by simply assigning a new, empty list).

EDIT: My assumptions are

• only one thread adds elements
• forgotten elements are okay (i.e. race condition between clearing and adding a new element), as long as the clear operation succeeds without problems
• .NET 2.0

Answer 1

There are two possibilities for problems here:

• Newly added items could end up being forgotten immediately, because you clear out and create a new list. Is that an issue? Basically, if AddElement and ClearList are called at the same time, you have a race condition: either the element will end up in the new list, or in the old (forgotten) one.
• List<T> isn't safe for multi-threaded mutation, so if two different threads call AddElement at the same time the results aren't guaranteed

Given that you're accessing a shared resource, I would personally hold a lock while accessing it. You'll still need to consider the possibility of clearing the list immediately before/after adding an item though.

EDIT: My comment about it being okay if you're only adding from one thread was already somewhat dubious, for two reasons:

• It's possible (I think!) that you could end up trying to add to a List<T> which hadn't been fully constructed yet. I'm not sure, and the .NET 2.0 memory model (as opposed to the one in the ECMA specification) may be strong enough to avoid that, but it's tricky to say.
• It's possible that the adding thread wouldn't "see" the change to the list variable immediately, and still add to the old list. Indeed, without any synchronization, it could see the old value forever

When you add "iterating in the GUI" into the mix it gets really tricky - because you can't change the list while you're iterating. The simplest solution to this is probably to provide a method which returns a copy of the list, and the UI can safely iterate over that:

class A
{
    private List<SomeClass> list;
    private readonly object listLock = new object();

    private void ClearList()
    {
        lock (listLock)
        {
            list = new List<SomeClass>();
        }
    }

    private void AddElement()
    {
        lock (listLock)
        {
            list.Add(new SomeClass(...));
        }
    }

    private List<SomeClass> CopyList()
    {
        lock (listLock)
        {
            return new List<SomeClass>(list);
        }
    }

}

Answer 2

Yes - it is possible,. In fact, if these are genuinely being called at the same time, it is highly likely.

In addition, it is also likely to cause problems if two seperate calls to addElement occur at the same time.

For this sort of multithreading, you really need some sort of mutually exclusive lock around the list itself, so only one operation on the underlying list can be called at a time.

A crude locking strategy around this would help. Something like:

class A
{
    static object myLock = new object()
    List<SomeClass> list;

    private void clearList()
    {
        lock(myLock)
        {
          list = new List<SomeClass>();
        }

    }

    private void addElement()
    {
        lock(myLock)
        {
          list.Add(new SomeClass(...));
        }
    }
}

Answer 3

It is properly not a good thing to just make a new List when you want to clear it.

I assume you also assigned list in the constructor so you don't run into a null-pointer exception.

If you clear and elements is added, they can be added to the old list which I assume is fine? BUT if two elements is added at the same time, you can run into problems.

Look into .Net 4 new collections to handle multithreading tasks :)

ADDITION: Look into the namespace System.Collections.Concurrent if you use .Net 4. There you will find: System.Collections.Concurrent.ConcurrentBag<T> and many other nice collections :)

You should also note that lock can significantly poll down performance if you dont watch out.

Answer 4

If you use one instance of this class in multiple threads, yes. you will run into problems. All collections in the .Net framework (version 3.5 and lower) are NOT thread-safe. Specially when you start changing the collection while another thread is itterating over it.

Use locking and give out ´copies of´ collections in multithreaded environments, or if you can use .Net 4.0, use the new concurrent collections.

Answer 5

It is clear from the edits to your question that you do not really care about the usual culprits here - there are really no simultaneous calls to the methods of the same object.

Essentially you are asking if it is ok to assign the reference to your list while it is being accessed from a parallel thread.

As far as I understand it still can cause trouble. It all depends on how reference assignment is implemented on the hardware level. To be more precise whether this operation is atomic or not.

I think that as slim as it is there is still a chance, especially in multiprocessor environments, that the process will get corrupted reference because it was only partially updated when it was accessing it.

Answer 6

Collections in .NET (up to 3.5) are not thread-safe or non-blocking (parallel execution). You should implement yours by deriving from IList and use a ReaderWriterLockSlim for performing every action. For example, your Add method should look like this:

    public void Add(T item)
    {
        _readerWriterLockSlim.EnterWriteLock();
        try { _actualList.Add(item); }
        finally { _readerWriterLockSlim.ExitWriteLock(); }
    }

You must be aware of some concurrency tricks here. For example you must have a GetEnumerator which returns a new instance as an IList; not the actual list. Otherwise you will run into problems; which should look like:

    public IEnumerator<T> GetEnumerator()
    {
        List<T> localList;

        _lock.EnterReadLock();
        try { localList= new List<T>(_actualList); }
        finally { _lock.ExitReadLock(); }

        foreach (T item in localList) yield return item;
    }

and:

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return ((IEnumerable<T>)this).GetEnumerator();
    }

Note: When implementing thread-safe or parallel collections (and in fact every other class) DO NOT DERIVE FROM THE CLASS, BUT INTERFACE! Because there will be always problems related to internal structure of that class or some methods that are not virtual and you have to hide them and so on. If you have to do this, do it very carefully!