How to write copy-on-write list in .NET

Question

How to write a thread-safe list using copy-on-write model in .NET?

Below is my current implementation, but after lots of reading about threading, memory barriers, etc, I know that I need to be cautious when multi-threading without locks is involved. Could someone comment if this is the correct implementation?

class CopyOnWriteList
{
    private List<string> list = new List<string>();

    private object listLock = new object();

    public void Add(string item)
    {
        lock (listLock)
        {
            list = new List<string>(list) { item };
        }
    }

    public void Remove(string item)
    {
        lock (listLock)
        {
            var tmpList = new List<string>(list);
            tmpList.Remove(item);
            list = tmpList;
        }
    }

    public bool Contains(string item)
    {
        return list.Contains(item);
    }

    public string Get(int index)
    {
        return list[index];
    }
}

EDIT

To be more specific: is above code thread safe, or should I add something more? Also, will all thread eventually see change in list reference? Or maybe I should add volatile keyword on list field or Thread.MemoryBarrier in Contains method between accessing reference and calling method on it?

Here is for example Java implementation, looks like my above code, but is such approach also thread-safe in .NET?

And here is the same question, but also in Java.

Here is another question related to this one.

Answer 1

Yes, it is thread-safe:

1. Collection modifications in Add and Remove are done on separate collections, so it avoids concurrent access to the same collection from Add and Remove or from Add/Remove and Contains/Get.

2. Assignment of the new collection is done inside lock, which is just pair of Monitor.Enter and Monitor.Exit, which both do a full memory barrier as noted here, which means that after the lock all threads should observe the new value of list field.

Answer 2

Implementation is correct because reference assignment is atomic in accordance to Atomicity of variable references. I would add volatile to list.

Answer 3

Your approach looks correct, but I'd recommend using a string[] rather than a List<string> to hold your data. When you're adding an item, you know exactly how many items are going to be in the resulting collection, so you can create a new array of exactly the size required. When removing an item, you can grab a copy of the list reference and search it for your item before making a copy; if it turns out that the item doesn't exist, there's no need to remove it. If it does exist, you can create a new array of the exact required size, and copy to the new array all the items preceding or following the item to be removed.

Another thing you might want to consider would be to use a int[1] as your lock flag, and use a pattern something like:

static string[] withAddedItem(string[] oldList, string dat)
{
  string[] result = new string[oldList.Length+1];      
  Array.Copy(oldList, result, oldList.Length);
  return result;
}
int Add(string dat) // Returns index of newly-added item
{
  string[] oldList, newList;
  if (listLock[0] == 0)
  {
    oldList  = list;
    newList = withAddedItem(oldList, dat);
    if (System.Threading.Interlocked.CompareExchange(list, newList, oldList) == oldList)
      return newList.Length;
  }
  System.Threading.Interlocked.Increment(listLock[0]);
  lock (listLock)
  {
    do
    {
      oldList  = list;
      newList = withAddedItem(oldList, dat);
    } while (System.Threading.Interlocked.CompareExchange(list, newList, oldList) != oldList);
  }
  System.Threading.Interlocked.Decrement(listLock[0]);
  return newList.Length;
}

If there is no write contention, the CompareExchange will succeed without having to acquire a lock. If there is write contention, writes will be serialized by the lock. Note that the lock here is neither necessary nor sufficient to ensure correctness. Its purpose is to avoid thrashing in the event of write contention. It is possible that thread #1 might get past its first "if" test, and get task task-switched out while many other threads simultaneously try to write the list and start using the lock. If that occurs, thread #1 might then "surprise" the thread in the lock by performing its own CompareExchange. Such an action would result in the lock-holding thread having to waste time making a new array, but that situation should arise rarely enough that the occasional cost of an extra array copy shouldn't matter.