27

I have a Generic List as below

public static readonly List<Customer> Customers = new List<Customer>();

I'm using the below methods for it:

.Add
.Find
.FirstOrDefault

The last 2 are LINQ extensions.

I'd need to make this thread-safe to be able to run multiple instances of the container class.

How to achieve that?

afuzzyllama
  • 6,538
  • 5
  • 47
  • 64
The Light
  • 26,341
  • 62
  • 176
  • 258
  • To get it clear, you want to use each instance from multiple threads simultaneously? Will multiple threads mutate the list? –  Apr 03 '12 at 14:26
  • Take a look at concurrency: http://geekswithblogs.net/BlackRabbitCoder/archive/2011/02/10/c.net-little-wonders-the-concurrent-collections-1-of-3.aspx – Silvermind Apr 03 '12 at 14:27

8 Answers8

42

If those are the only functions you are using on List<T> then the easiest way is to write a quick wrapper that synchronizes access with a lock

class MyList<T> { 
  private List<T> _list = new List<T>();
  private object _sync = new object();
  public void Add(T value) {
    lock (_sync) {
      _list.Add(value);
    }
  }
  public bool Find(Predicate<T> predicate) {
    lock (_sync) {
      return _list.Find(predicate);
    }
  }
  public T FirstOrDefault() {
    lock (_sync) {
      return _list.FirstOrDefault();
    }
  }
}

I highly recommend the approach of a new type + private lock object. It makes it much more obvious to the next guy who inherits your code what the actual intent was.

Also note that .Net 4.0 introduced a new set of collections specifically aimed at being used from multiple threads. If one of these meets your needs I'd highly recommend using it over rolling your own.

  • ConcurrentStack<T>
  • ConcurrentQueue<T>
JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
  • 1
    I think it is better to use a ReaderWriterLock in such a scenario, to allow multiple threads reading the collection at the same time. – Hari Apr 03 '12 at 14:33
  • 1
    @Hari it depends. A `ReaderWriterLock` is only more efficient if the reads are at least double the writes other wise a plain old lock is faster. OP would have to provide the context to say this was or wasn't the case – JaredPar Apr 03 '12 at 14:36
  • 1
    One thing that's slightly ugly with that code is that the lock is held while running outside code for the `Find` predicate. If the only three methods are as indicated, one could pretty easily build an inherently-threadsafe structure. For example, one could have an array of ~32 array references. The first would hold an array of size e.g. 16, the next an array of size 32, then one of size 64, etc. Writes would require locks, but reads or enumerations would not (an enumeration would return the items that existed when the enumeration was started, and no such item would ever be disturbed). – supercat Apr 03 '12 at 16:18
  • why don't you use the list itself as lock? (obviusly it should be final, so no "reallocation by new" error) – Lesto Feb 20 '15 at 12:01
  • @Lesto Locking a non-private object is an habit I personnally frown upon, as there's no telling when somebody else will have the unfortunate idea to use the exact same instance for locking in another thread. – Crono Apr 12 '18 at 22:47
  • @Crono i suggetstd to lock on "_list", wich is private, not on "this", if i understand correctly your affirmation – Lesto Apr 14 '18 at 10:40
11

To expand on @JaradPar's answer, here is a full implementation with a few extra features, as described in the summary

/// <summary>
/// a thread-safe list with support for:
/// 1) negative indexes (read from end).  "myList[-1]" gets the last value
/// 2) modification while enumerating: enumerates a copy of the collection.
/// </summary>
/// <typeparam name="TValue"></typeparam>
public class ConcurrentList<TValue> : IList<TValue>
{
    private object _lock = new object();
    private List<TValue> _storage = new List<TValue>();
    /// <summary>
    /// support for negative indexes (read from end).  "myList[-1]" gets the last value
    /// </summary>
    /// <param name="index"></param>
    /// <returns></returns>
    public TValue this[int index]
    {
        get
        {
            lock (_lock)
            {
                if (index < 0)
                {
                    index = this.Count - index;
                }
                return _storage[index];
            }
        }
        set
        {
            lock (_lock)
            {
                if (index < 0)
                {
                    index = this.Count - index;
                }
                _storage[index] = value;
            }
        }
    }

    public void Sort()
    {
        lock (_lock)
        {
            _storage.Sort();
        }
    }

    public int Count
    {
        get
        {
            lock (_lock) return _storage.Count;
        }
    }

    bool ICollection<TValue>.IsReadOnly
    {
        get
        {
            return ((IList<TValue>)_storage).IsReadOnly;
        }
    }

    public void Add(TValue item)
    {
        lock (_lock)
        {
            _storage.Add(item);
        }
    }

    public void Clear()
    {
        lock (_lock)
        {
            _storage.Clear();
        }
    }

    public bool Contains(TValue item)
    {
        lock (_lock)
        {
            return _storage.Contains(item);
        }
    }

    public void CopyTo(TValue[] array, int arrayIndex)
    {
        lock (_lock)
        {
            _storage.CopyTo(array, arrayIndex);
        }
    }

    public int IndexOf(TValue item)
    {
        lock (_lock)
        {
            return _storage.IndexOf(item);
        }
    }

    public void Insert(int index, TValue item)
    {
        lock (_lock)
        {
            _storage.Insert(index, item);
        }
    }

    public bool Remove(TValue item)
    {
        lock (_lock)
        {
            return _storage.Remove(item);
        }
    }

    public void RemoveAt(int index)
    {
        lock (_lock)
        {
            _storage.RemoveAt(index);
        }
    }

    public IEnumerator<TValue> GetEnumerator()
    {
        lock (_lock)
        {
            return _storage.ToArray().AsEnumerable().GetEnumerator();
        }
    }
    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
JasonS
  • 7,443
  • 5
  • 41
  • 61
  • FYI: if you need more concurrent read performance, could use a ReadWriteLockSlim instead of the lock. but before you bother, measure and make sure this is your performance bottleneck! – JasonS Oct 12 '15 at 16:38
  • 3
    Only a question... Why the double lock in public IEnumerator GetEnumerator()? Thanks. – Adam Calvet Bohl Apr 19 '16 at 10:32
  • Thank you for posting this. – joelc Jun 14 '16 at 21:51
  • @AdamCalvetBohl sorry, that's a copy/paste typo :) it doesn't cause any bug thankfully! – JasonS Jun 17 '16 at 20:02
  • 2
    Hi @JasonS just a heads-up I had to change public ```IEnumerator GetEnumerator()``` to, inside the lock, ```return _storage.GetEnumerator();```. I was getting a strange Unable To Cast Object Of Type 'SZArrayEnumerator' To Type message. Thanks! – joelc Jun 20 '16 at 00:10
  • 1
    @Joelc makes a good point. public IEnumerator GetEnumerator() .... needs to be updated in order for it to work. – TheLegendaryCopyCoder Feb 21 '17 at 12:02
  • 1
    sorry, it worked in 2015, i guess the latest .net makes them incompatible. FYI @joelc i think you might have a race condition with your work around, you need to clone the storage and return an enumerator of the clone. – JasonS Mar 06 '17 at 23:40
  • 1
    This should be the answer considering it satisfies the D in SOLID. – aj.toulan Jan 23 '18 at 20:57
  • 1
    @JasonS I made an improvement in the `GetEnumerator()` method if you don't mind (removed the double `lock`). I also added `lock` protection for the `Count` property, because it's better to be safe than sorry! – Theodor Zoulias Apr 24 '22 at 21:07
10

If you're using version 4 or greater of the .NET framework you can use the thread-safe collections.

You can replace List<T> with ConcurrentBag<T>:

namespace Playground.Sandbox
{
    using System.Collections.Concurrent;
    using System.Threading.Tasks;

    public static class Program
    {
        public static void Main()
        {
            var items = new[] { "Foo", "Bar", "Baz" };
            var bag = new ConcurrentBag<string>();
            Parallel.ForEach(items, bag.Add);
        }
    }
}
Albireo
  • 10,977
  • 13
  • 62
  • 96
2

You will need to use locks in every place where the collection gets modified or iterated over.

Either that or use one of the new thread-safe data structures, like ConcurrentBag.

Tudor
  • 61,523
  • 12
  • 102
  • 142
1

Never use ConcurrangBag for ordered data. Use Array instead

Moctar Haiz
  • 179
  • 1
  • 6
1

Use the lock keyword when you manipulate the collection, ie: your Add/Find:

lock(Customers) {
    Customers.Add(new Customer());
}
Darren
  • 68,902
  • 24
  • 138
  • 144
0

Ok, so I had to completely rewrite my answer. After 2 days of testing I have to say that the JasonS's code has some defects, I guess because of Enumerators. While one thread uses foreach, and the other other changes the list, it throws exceptions.

So I found this answer, and it works for me fine the last 48 hours non-stop, I guess more than 100k threads were created in my application, and used that lists.

The only thing I changed - I've moved entering the locks outside the try-finally section. Read here about the possible exceptions. Also, if you will read MSDN, they have the same approach.

But, as were mentioned in link below, List can not be 100% thread safe, probably that is why there is no default ConcurentList implementation in c#.

Community
  • 1
  • 1
Max
  • 121
  • 7
0

Make your Action as accessible by one only by using lock on any private object

Refer to : Thread Safe Generic Queue Class

http://www.codeproject.com/Articles/38908/Thread-Safe-Generic-Queue-Class

JSJ
  • 5,653
  • 3
  • 25
  • 32
  • The attached .zip is unavailable in this article. Too bad, I wanted a thread safe List<>. – Cœur Jun 17 '14 at 09:55