Creating Thread Safe List using Reader Writer Lock

问题:

Completely editing the earlier version, Can the following implementation be the Thread Safe List implementation. I just need to know whether it would truly thread safe or not, I know performance wise there would still be issues. Currently version is using ReaderWriterLockSlim, I have another implementation using the Lock, doing the same job

using System.Collections.Generic; using System.Threading;

/// <summary>
/// Thread safe version of the List using ReaderWriterLockSlim 
/// </summary>
/// <typeparam name="T"></typeparam>
public class ThreadSafeListWithRWLock<T> : IList<T>
{
    // Internal private list which would be accessed in a thread safe manner
    private List<T> internalList;

    // ReaderWriterLockSlim object to take care of thread safe acess between multiple readers and writers
    private readonly ReaderWriterLockSlim rwLockList;

    /// <summary>
    /// Public constructor with variable initialization code
    /// </summary>
    public ThreadSafeListWithRWLock()
    {
        internalList = new List<T>();

        rwLockList = new ReaderWriterLockSlim();
    }

    /// <summary>
    /// Get the Enumerator to the Thread safe list
    /// </summary>
    /// <returns></returns>
    public IEnumerator<T> GetEnumerator()
    {
        return Clone().GetEnumerator();
    }

    /// <summary>
    /// System.Collections.IEnumerable.GetEnumerator implementation to get the IEnumerator type
    /// </summary>
    /// <returns></returns>
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return Clone().GetEnumerator();
    }

    /// <summary>
    /// Clone method to create an in memory copy of the Thread safe list
    /// </summary>
    /// <returns></returns>
    public List<T> Clone()
    {
        List<T> clonedList = new List<T>();

        rwLockList.EnterReadLock();

        internalList.ForEach(element => { clonedList.Add(element); });            

        rwLockList.ExitReadLock();

        return (clonedList);
    }

    /// <summary>
    /// Add an item to Thread safe list
    /// </summary>
    /// <param name="item"></param>
    public void Add(T item)
    {
        rwLockList.EnterWriteLock();

        internalList.Add(item);

        rwLockList.ExitWriteLock();
    }

    /// <summary>
    /// Remove an item from Thread safe list
    /// </summary>
    /// <param name="item"></param>
    /// <returns></returns>
    public bool Remove(T item)
    {
        bool isRemoved;

        rwLockList.EnterWriteLock();

        isRemoved = internalList.Remove(item);

        rwLockList.ExitWriteLock();

        return (isRemoved);
    }

    /// <summary>
    /// Clear all elements of Thread safe list
    /// </summary>
    public void Clear()
    {
        rwLockList.EnterWriteLock();

        internalList.Clear();

        rwLockList.ExitWriteLock();
    }

    /// <summary>
    /// Contains an item in the Thread safe list
    /// </summary>
    /// <param name="item"></param>
    /// <returns></returns>
    public bool Contains(T item)
    {
        bool containsItem;

        rwLockList.EnterReadLock();

        containsItem = internalList.Contains(item);

        rwLockList.ExitReadLock();

        return (containsItem);
    }

    /// <summary>
    /// Copy elements of the Thread safe list to a compatible array from specified index in the aray
    /// </summary>
    /// <param name="array"></param>
    /// <param name="arrayIndex"></param>
    public void CopyTo(T[] array, int arrayIndex)
    {
        rwLockList.EnterReadLock();

        internalList.CopyTo(array,arrayIndex);

        rwLockList.ExitReadLock();
    }

    /// <summary>
    /// Count elements in a Thread safe list
    /// </summary>
    public int Count
    {
        get
        {
            int count;

            rwLockList.EnterReadLock();

            count = internalList.Count;

            rwLockList.ExitReadLock();

            return (count);
        }
    }

    /// <summary>
    /// Check whether Thread safe list is read only
    /// </summary>
    public bool IsReadOnly
    {
        get { return false; }
    }

    /// <summary>
    /// Index of an item in the Thread safe list
    /// </summary>
    /// <param name="item"></param>
    /// <returns></returns>
    public int IndexOf(T item)
    {
        int itemIndex;

        rwLockList.EnterReadLock();

        itemIndex = internalList.IndexOf(item);

        rwLockList.ExitReadLock();

        return (itemIndex);
    }

    /// <summary>
    /// Insert an item at a specified index in a Thread safe list
    /// </summary>
    /// <param name="index"></param>
    /// <param name="item"></param>
    public void Insert(int index, T item)
    {
      rwLockList.EnterWriteLock();

      if (index <= internalList.Count - 1 && index >= 0)
        internalList.Insert(index,item);

      rwLockList.ExitWriteLock();
    }

    /// <summary>
    /// Remove an item at a specified index in Thread safe list
    /// </summary>
    /// <param name="index"></param>
    public void RemoveAt(int index)
    {
       rwLockList.EnterWriteLock();

       if (index <= internalList.Count - 1 && index >= 0)
        internalList.RemoveAt(index);

       rwLockList.ExitWriteLock();
    }

    /// <summary>
    /// Indexer for the Thread safe list
    /// </summary>
    /// <param name="index"></param>
    /// <returns></returns>
    public T this[int index] 
    {
        get
        {
            T returnItem = default(T);

           rwLockList.EnterReadLock();

           if (index <= internalList.Count - 1 && index >= 0)
               returnItem = internalList[index];              

           rwLockList.ExitReadLock();

            return (returnItem);
        }
        set
        {
            rwLockList.EnterWriteLock();

            if (index <= internalList.Count - 1 && index >= 0)
                internalList[index] = value;

            rwLockList.ExitWriteLock();
        }
    }
}

回答1:

Implementing a custom List<T> that encapsulates thread-safety is rarely worth the effort. You're likely best off just using lock whenever you access the List<T>.

But being in a performance intensive industry myself there have been cases where this becomes a bottleneck. The main drawback to lock is the possibility of context switching which is, relatively speaking, extremely expensive both in wall clock time and CPU cycles.

The best way around this is using immutability. Have all readers access an immutable list and writers "update" it using Interlocked operations to replace it with a new instance. This is a lock-free design that makes reads free of synchronization and writes lock-free (eliminates context switching).

I will stress that in almost all cases this is overkill and I wouldn't even consider going down this path unless you're positive you need to and you understand the drawbacks. A couple of the obvious ones are readers getting point-in-time snapshots and wasting memory creating copies.

ImmutableList from Microsoft.Bcl.Immutable is also worth a look. It's entirely thread-safe.

回答2:

That is not threadsafe.

The method GetEnumerator() will not retain any lock after the enumerator has been returned, so any thread would be free to use the returned enumerator without any locking to prevent them from doing so.

In general, trying to create a threadsafe list type is very difficult.

See this StackOverflow thread for some discussion: No ConcurrentList<T> in .Net 4.0?

回答3:

If you are trying to use some kind of reader/writer lock, rather than a simple locking scheme for both reading and writing, your concurrent reads probably greatly outnumber your writes. In that case, a copy-on-write approach, as suggested by Zer0, can be appropriate.

In an answer to a related question I posted a generic utility function that helps turning any modification on any data structure into a thread-safe and highly concurrent operation.

Code

static class CopyOnWriteSwapper
{
    public static void Swap<T>(ref T obj, Func<T, T> cloner, Action<T> op)
        where T : class
    {
        while (true)
        {
            var objBefore = Volatile.Read(ref obj);
            var newObj = cloner(objBefore);
            op(newObj);
            if (Interlocked.CompareExchange(ref obj, newObj, objBefore) == objBefore)
                return;
        }
    }
}

Usage

CopyOnWriteSwapper.Swap(ref _myList,
    orig => new List<string>(orig),
    clone => clone.Add("asdf"));

More details about what you can do with it, and a couple caveats can be found in the original answer.