How make custom Thread Safe Generic List return the whole list in C#?

Question

I am a threading noob and I am trying to write a custom thread safe generic list class in C# (.NET 3.5 SP1). I've read Why are thread safe collections so hard?. After reviewing the requirements of the class I think I only need to safely add to the list and return the list. The example shows pretty much all I want except it lacks the return list method therefore I've written my own public method like below:

Update: based on suggestions given I've reviewed my requirements and therefore simplified the class to as below:

public sealed class ThreadSafeList<T>
{
    private readonly IList<T> list = new List<T>();
    private readonly object lockable = new object();

    public void Add(T t)
    {
        lock (lockable)
        {
            list.Add(t);
        }
    }

    public IList<T> GetSnapshot()
    {
        IList<T> result;
        lock (lockable)
        {
            result = new List<T>(list);
        }
        return result;
    }
}

Answer 1

In my experience you have to use your brain when it comes to thread-safety and not rely on solutions such as these. in short, it depends on what the receiver of the list is going to be doing with it.

Answer 2

The Translate() method looks correct. Using the lock you are preventing others from adding or otherwise modifying your list while you are in Translate/AddRange.

I think there might be a problem with your IsReadyOnly property though. You use a lock when reading/writing the property internally. But there also is a public getter which is not locked. It might happen that thread 1 calls MarkAsReadOnly while a second thread might still get false when looking at IsReadOnly. I'd use a normal property instead and either lock in the getter or use a volatile bool field.

Answer 3

Agree with @jrista. There's a semantics issue you need to resolve, and why is it called Translate()? What is the intent?

A - current code - return a read-only wrapper of the internal list

return new ReadOnlyCollection<T>(list);

You still have threading issues if the original list is changed if another thread is iterating over the list. As long as you're aware of this, it isn't a huge issue.

B - a read-only copy.

return new List<T>(list).AsReadOnly();

This list has no threading issues because nothing modifies the new list. The only reference held is by the ReadOnlyCollection<T> wrapper.

C - a normal (writable) copy

return new List<T>(list);

Returns a new list, and the caller can do what they wish to their list without affecting the original list, and changes to the original list do not affect this list.

---

Does it matter if another consumer grabs a copy of the list and then modifies their copy? Do consumers need to see changes to the list? Do you just need a thread-safe enumerator?

public IEnumerator<T> ThreadSafeEnumerator()
{
    List<T> copy;
    lock(lockable)
        copy = new List<T>(list);

    foreach (var value in copy)
        yield return value;
}