180

I want an implementation of List<T> as a property which can be used thread-safely without any doubt.

Something like this:

private List<T> _list;

private List<T> MyT
{
    get { // return a copy of _list; }
    set { _list = value; }
}

It seems still I need to return a copy (cloned) of collection so if somewhere we are iterating the collection and at the same time the collection is set, then no exception is raised.

How to implement a thread-safe collection property?

dsolimano
  • 8,870
  • 3
  • 48
  • 63
Xaqron
  • 29,931
  • 42
  • 140
  • 205
  • 8
    use locks, that should do it. – atoMerz May 03 '11 at 19:05
  • Can use use a thread-safe implementation of `IList` (vs `List`)? – Greg May 03 '11 at 19:13
  • 3
    Have you checked [SynchronizedCollection](https://msdn.microsoft.com/en-us/library/ms668265(v=vs.110).aspx) ? – Roi Shabtai May 23 '16 at 07:22
  • 1
    Use BlockingCollection or ConcurrentDictionary – kumar chandraketu Sep 18 '18 at 04:28
  • What operations you need to do with object behind the property? Is it possible that you don't need everything that `List` implements? If yes, then could you please provide an interface that you need instead of asking about everything that `List` already have? – Victor Yarema Nov 16 '18 at 18:39
  • This is probably the question all good programmers sooner or later ask in their life. Using lock(…) around all List operations seems the right answer. – thomasgalliker Apr 15 '22 at 09:40
  • Perhaps the question is a bit misleading, as you start asking for a thread-safe implementation of List, but end asking for a thread-safe collection. These are two different things. A collection can be counted and enumerated, a list can be modified, accessed with a numeric index, ordered, etc. – Siderite Zackwehdex Apr 29 '23 at 07:13

15 Answers15

233

If you are targetting .Net 4 there are a few options in System.Collections.Concurrent Namespace

You could use ConcurrentBag<T> in this case instead of List<T>

Bala R
  • 107,317
  • 23
  • 199
  • 210
  • 9
    Like List and unlike Dictionary, ConcurrentBag accepts duplicates. – The Light May 24 '12 at 09:17
  • 170
    `ConcurrentBag` is unordered collection, so unlike `List` it does not guarantee ordering. Also you cannot access items by index. – Radek Stromský Mar 07 '13 at 13:56
  • 17
    @RadekStromský is right, and in the case you wanna an ordered concurrent list, you could try [ConcurrentQueue (FIFO)](http://msdn.microsoft.com/en-us/library/dd267265(v=vs.110).aspx) or [ConcurrentStack (LIFO)](http://msdn.microsoft.com/en-us/library/dd267331(v=vs.110).aspx). – Caio Cunha Dec 30 '13 at 21:14
  • 5
    And `ConcurrentBag` creates copy of its collection for each thread so this might be misleading. Also operation such as removing and adding behaves diffrent. – Fka Sep 15 '15 at 17:22
  • 13
    Maybe [SynchronizedCollection](https://msdn.microsoft.com/en-us/library/ms668265(v=vs.110).aspx) ? – Roi Shabtai May 23 '16 at 07:24
  • 19
    ConcurrentBag doesn't implement IList and is not actually thread safe version of List – Vasyl Zvarydchuk Apr 03 '17 at 20:55
  • 2
    @VasylZvarydchuk - sure technically it's not a List. But it's exactly what most people are looking for when they are using List but need something thread-safe. – Don Cheadle Jan 11 '19 at 21:34
  • 1
    @DonCheadle Except you can't remove items from it either. I've never yet found a case where it was useful. – 15ee8f99-57ff-4f92-890c-b56153 Aug 11 '21 at 20:35
  • https://www.c-sharpcorner.com/article/thread-safe-concurrent-collection-in-C-Sharp/ – Ramesh Dutta Sep 14 '22 at 14:28
  • ConcurrentBag does not have remove function. and if you need to remove from a list it is not recomended – ghane nikraftar Oct 13 '22 at 08:03
129

Even as it got the most votes, one usually can't take System.Collections.Concurrent.ConcurrentBag<T> as a thread-safe replacement for System.Collections.Generic.List<T> as it is (Radek Stromský already pointed it out) not ordered.

But there is a class called System.Collections.Generic.SynchronizedCollection<T> that is already since .NET 3.0 part of the framework, but it is that well hidden in a location where one does not expect it that it is little known and probably you have never ever stumbled over it (at least I never did).

SynchronizedCollection<T> is compiled into assembly System.ServiceModel.dll (which is part of the client profile but not of the portable class library).

John Smith
  • 7,243
  • 6
  • 49
  • 61
Christoph
  • 3,322
  • 2
  • 19
  • 28
  • 1
    Additional helpful discussion of this option: http://stackoverflow.com/a/4655236/12484 – Jon Schneider May 19 '16 at 15:54
  • 7
    @denfromufa it look likes they added this in .net core 2.0 https://learn.microsoft.com/en-gb/dotnet/api/system.collections.generic.synchronizedcollection-1?view=netframework-4.7.1#Applies_to – Cirelli94 Mar 07 '18 at 15:00
  • 3
    ConcurrentBag is not a replacement of list. It doesn't behaves like list, You cannot remove elements like lists. In lists you can specify the item to be removed, you cannot do this with concurrent bags – amarnath chatterjee Oct 28 '21 at 08:21
  • Unfortunately this isn't completely thread safe. Enumarations are not thread safe and that's one of the main reasons why one would pick this over other types. – Ivan Ičin Mar 16 '22 at 13:46
  • I just broke my website using concurrentbag as a replacement to list. I thought it may have failed but I wasn’t sure. Would’ve been a lot easier if there was just a concurrent list like thing. I think I’ll try the stack next :) – Simon_Weaver Jun 23 '22 at 18:05
22

I would think making a sample ThreadSafeList class would be easy:

public class ThreadSafeList<T> : IList<T>
{
    protected List<T> _internalList = new List<T>();

    // Other Elements of IList implementation

    public IEnumerator<T> GetEnumerator()
    {
        return Clone().GetEnumerator();
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return Clone().GetEnumerator();
    }

    protected static object _lock = new object();

    public List<T> Clone()
    {
        List<T> newList = new List<T>();

        lock (_lock)
        {
            _internalList.ForEach(x => newList.Add(x));
        }

        return newList;
    }
}

You simply clone the list before requesting an enumerator, and thus any enumeration is working off a copy that can't be modified while running.

maxp
  • 24,209
  • 39
  • 123
  • 201
Tejs
  • 40,736
  • 10
  • 68
  • 86
  • 1
    Isn't this a shallow clone? If `T` is a reference type, won't this just return a new list containing references to all the original objects? If that is the case, this approach could still cause threading trouble since the list objects could be accessed by multiple threads through different "copies" of the list. – Joel B Jan 02 '13 at 22:09
  • 4
    Correct, it is a shallow copy. The point was to simply have a cloned set that would be safe to iterate over (so `newList` does not have any items added or removed which would invalidate the enumerator). – Tejs Jan 03 '13 at 18:47
  • Roger that. This is a really elegant solution to this problem. After some serious thinking/Googl-ing I haven't really turned up anything else that comes close to this in terms of balancing safety/simplicity. Thanks for posting this! – Joel B Jan 03 '13 at 20:02
  • 13
    Should the _lock be static? – Mike Ward Nov 18 '13 at 16:43
  • 5
    Another thought. Is this implementation threadsafe for multiple writers? If not, maybe it should be called a ReadSafeList. – Mike Ward Nov 18 '13 at 16:49
  • 11
    @MikeWard - I don't think it should be, **all** instance will lock when **any** instance is being cloned! – Josh M. Jan 16 '14 at 13:09
  • r-h made a similar implementation but where the (private?) method `IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); }` will not return a `Clone()`. Is that an improvement or an issue? I don't know which one is best. – Cœur Jun 17 '14 at 21:56
  • 1
    Yes - locking on the internal list directly would be better; otherwise all instances lock when a single instance clones. As for the enumerator call, it's cloned so that when enumerating, if a new element is added to the list while another thread is enumerating, it doesn't blow the enumerator out because the underlying collection was modified during enumeration. – Tejs Jun 18 '14 at 15:27
  • This is not a clone, it's a copy. You **really** should rename it. Maybe create a copy constructor, like `List` has. – ANeves Feb 05 '15 at 16:10
  • 1
    Your `Clone()` method could simply be `lock(_lock) { return new List(_internalList); }` – Mr Anderson Aug 30 '16 at 00:53
  • 1
    On very large List Collections, Clone() should be avoided. Resource use should be thought about. Taking advantage of CODE Examples https://referencesource.microsoft.com/#mscorlib/system/Collections/Concurrent/ConcurrentDictionary.cs should be a priority. – Rusty Nail Jan 04 '18 at 21:07
  • 1
    Why "reinvent the square wheel" when MS has now made many different types of Concurrent alternatives. – bytedev Sep 21 '18 at 14:31
19

Even accepted answer is ConcurrentBag, I don't think it's real replacement of list in all cases, as Radek's comment to the answer says: "ConcurrentBag is unordered collection, so unlike List it does not guarantee ordering. Also you cannot access items by index".

So if you use .NET 4.0 or higher, a workaround could be to use ConcurrentDictionary with integer TKey as array index and TValue as array value. This is recommended way of replacing list in Pluralsight's C# Concurrent Collections course. ConcurrentDictionary solves both problems mentioned above: index accessing and ordering (we can not rely on ordering as it's hash table under the hood, but current .NET implementation saves order of elements adding).

tytyryty
  • 741
  • 7
  • 17
  • I didn't down-vote and there is no reason for it IMO. You are right but the concept is already mentioned in some answers. To me, the point was there is an new thread-safe collection in .NET 4.0 which I was not aware of. Not sure used Bag or Collection for the situation. +1 – Xaqron Mar 31 '17 at 23:18
  • 4
    This answer has several problems: 1) `ConcurrentDictionary` is a dictionary, not a list. 2) It is not guaranteed to preserver order, as your own answer states, which contradicts your stated reason for posting an answer. 3) It links to a *video* without bringing the relevant quotes into this answer (which might not be in concordance with their licensing anyway). – jpmc26 May 18 '18 at 17:47
  • 1
    You can't rely on things like `current implementation` if it is not explicitly guaranteed by the documentation. Implementation may change any time without a notice. – Victor Yarema Nov 14 '18 at 18:55
  • 1
    @jpmc26, yes it is not a full replacement for List of course, however the same is with ConcurrentBag as an accepted answer - it is not strict replacement of List but work around. To reply your concerns: 1) ConcurrentDictionary is a dictionary not a list you are right, however list has array behind, which can index in O(1) same as dictionary with int as a key 2) yes order is not guaranteed by doc (even though it is preserved), but accepted ConcurrentBag cannot guarantee order as well in multithreaded scenarios – tytyryty Oct 21 '19 at 08:53
  • 1
    That suggestion has potential in my opinion. If Dictionary.Count is used as a key (in case there are no deletes), any thread may add values like this `while (!myDict.TryAdd(myDict.Count, myValue)) { }` (or use an atomic increment of a counter in case there might be deletes). This would guarantee the values can be brought into the original order when retrieving them. – Christoph Dec 11 '19 at 16:48
19

C#'s ArrayList class has a Synchronized method.

var threadSafeArrayList = ArrayList.Synchronized(new ArrayList());

This returns a thread safe wrapper around any instance of IList. All operations need to be performed through the wrapper to ensure thread safety.

jpmc26
  • 28,463
  • 14
  • 94
  • 146
Hani Nakhli
  • 351
  • 2
  • 9
11

In .NET Core (any version), you can use ImmutableList, which has all the functionality of List<T>.

JotaBe
  • 38,030
  • 8
  • 98
  • 117
  • Yeah, but you still need to wrap it into locks - it's immutable, so when you modify it you need to re-assign the new list, which is not thread-safe. – Mr Patience Nov 06 '22 at 08:25
  • @MrPatience Nope! The concurrency problems of a List is that a thread modifies it while another thread is iterating it. In this case if a thread is iterating the list, and another thread modifies it, the iterated list is unchanged, and a new modified list is created, so there is no concurrency problem. Remember that the objects are passed by reference, so the iterating thread still has the reference to the original, unchanged list. If you don't believe it, just check the docs: https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablelist-1#thread-safety – JotaBe Nov 07 '22 at 12:22
9

In reply to all of the answers since my original one:

People found these solutions:

  • use SynchronizedCollection - the source code is almost identical to the SynchronizedList below, with the same features and issues
  • use ArrayList.Synchronized - this will return a SyncIList, which is the same thing as the SynchronizedList below, only not generic
  • using a thread safe form of a list, where on every access you clone it - I believe the implementation presented could be improved significantly using ThreadLocal or AsyncLocal, but it would still fail any performance tests
  • use various combinations of classes in the Collections.Concurrent namespace - these contains some good options for ICollection, but NOT for IList for index access
  • use a ConcurrentDictionary<int, T>, with indexes as keys, to emulate an IList - this is one of the best ideas I've seen, but it's not really in the spirit of an IList, which implies O(1) index read/append complexity and some complexity in insert/deletes, as well as an O(n) space complexity. Also, what about IndexOf and sorting operations?

Most complaints against the SynchronizedList class were related to:

  • slowness of the lock mechanism - the performance calculation varies wildly based on the scenario where the list is used, so this is a valid option for a vague requirement
  • using lock(object) and not using SemaphoreSlim - OK, this is my complaint :) but fixing the code to use it is trivial

A more complex system of locks can be implemented, like for individual rows, groups of rows, etc. That will start to look like implementing your own database, though. Writing a high performance collection is an art and is very tightly coupled to the specific scenario you want to use it in.

I still believe that for a general use scenario the simple solution here is the most versatile.

The C5 collection library, which is one of the inspirations for great collection design, handles concurrency thus:

  • no concurrent collections (by design) because they feel a simple lock mechanism can be implemented, but scenarios get very complex when multiple collections are getting accessed at the same time
  • "A tree-based collection can be safely enumerated while modifying it" - where they recommend "pattern 66": "one can take a snapshot of the tree and then enumerate the items of that snapshot, while modifying the original tree at the same time"

Original answer:

If you look at the source code for List of T (https://referencesource.microsoft.com/#mscorlib/system/collections/generic/list.cs,c66df6f36c131877) you will notice there is a class there (which is of course internal - why, Microsoft, why?!?!) called SynchronizedList of T. I am copy pasting the code here:

   [Serializable()]
    internal class SynchronizedList : IList<T> {
        private List<T> _list;
        private Object _root;

        internal SynchronizedList(List<T> list) {
            _list = list;
            _root = ((System.Collections.ICollection)list).SyncRoot;
        }

        public int Count {
            get {
                lock (_root) { 
                    return _list.Count; 
                }
            }
        }

        public bool IsReadOnly {
            get {
                return ((ICollection<T>)_list).IsReadOnly;
            }
        }

        public void Add(T item) {
            lock (_root) { 
                _list.Add(item); 
            }
        }

        public void Clear() {
            lock (_root) { 
                _list.Clear(); 
            }
        }

        public bool Contains(T item) {
            lock (_root) { 
                return _list.Contains(item);
            }
        }

        public void CopyTo(T[] array, int arrayIndex) {
            lock (_root) { 
                _list.CopyTo(array, arrayIndex);
            }
        }

        public bool Remove(T item) {
            lock (_root) { 
                return _list.Remove(item);
            }
        }

        System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() {
            lock (_root) { 
                return _list.GetEnumerator();
            }
        }

        IEnumerator<T> IEnumerable<T>.GetEnumerator() {
            lock (_root) { 
                return ((IEnumerable<T>)_list).GetEnumerator();
            }
        }

        public T this[int index] {
            get {
                lock(_root) {
                    return _list[index];
                }
            }
            set {
                lock(_root) {
                    _list[index] = value;
                }
            }
        }

        public int IndexOf(T item) {
            lock (_root) {
                return _list.IndexOf(item);
            }
        }

        public void Insert(int index, T item) {
            lock (_root) {
                _list.Insert(index, item);
            }
        }

        public void RemoveAt(int index) {
            lock (_root) {
                _list.RemoveAt(index);
            }
        }
    }

Personally I think they knew a better implementation using SemaphoreSlim could be created, but didn't get to it.

Siderite Zackwehdex
  • 6,293
  • 3
  • 30
  • 46
  • 3
    +1 Locking the whole collection (`_root`) in each access (read/write) makes this a slow solution. Maybe it is better for this class to remain internal. – Xaqron Aug 12 '18 at 20:38
  • 4
    This implementation is not thread-safe. It still throws "System.InvalidOperationException: 'Collection was modified; enumeration operation may not execute.'" – Raman Zhylich Nov 28 '18 at 19:17
  • 5
    That is not related to thread safety, but to the fact that you are iterating and changing the collection. The exception is thrown by the enumerator when it sees the list was changed. To get around this you need to implement your own IEnumerator or change the code so that it doesn't iterate and change the same collection at the same time. – Siderite Zackwehdex Nov 30 '18 at 11:41
  • 3
    It's not thread-safe because the collection _can_ be changed during the "synchronized" methods. That absolutely _is_ part of thread safety. Consider one thread calls `Clear()` after another calls `this[index]` but before the lock is activated. `index` is no longer safe to use and will throw an exception when it finally executes. – Suncat2000 Jan 30 '20 at 16:57
  • 1
    @Suncat2000 how is that going to happen? both the `Clear()` and `this[index]` methods are locking, and using the same lock object. Wouldn't the lock in `Clear()` prevent `this[index]` from executing before the list is cleared? – Kemuel Sanchez Apr 27 '23 at 17:54
  • 1
    @KemuelSanchez That is exactly my point. The code in `this[index]` will stop until `Clear()` completes, then will throw an exception because `index` is no longer valid. There is no check to see if `index` is valid before using it. It may be technically thread-safe, but it isn't much friendlier than not using locks. – Suncat2000 Apr 28 '23 at 18:23
  • 1
    Well, you could as easily cause the same issue in single thread by having a reference to the list, then calling another function that clears the list and then trying to access an element directly by index. index-based access should be guarding against those exceptions IMHO, but I do see your point of use-friendliness. – Kemuel Sanchez Apr 29 '23 at 04:40
  • I have updated my answer based on the comments and other answers here. I don't think there is a better solution to the problem without designing an entirely different internal functionality of the List, which I believe would be against the intentions of the OP. – Siderite Zackwehdex Apr 29 '23 at 07:06
2

It seems like many of the people finding this are wanting a thread safe indexed dynamically sized collection. The closest and easiest thing I know of would be.

System.Collections.Concurrent.ConcurrentDictionary<int, YourDataType>

This would require you to ensure your key is properly incremented if you want normal indexing behavior. If you are careful .count() could suffice as the key for any new key value pairs you add.

Alex P.
  • 1,140
  • 13
  • 27
user2163234
  • 527
  • 5
  • 9
2

I would suggest anyone dealing with a List<T> in multi-threading scenarios to take look at Immutable Collections in particular the ImmutableArray.

I've found it very useful when you have:

  1. Relatively few items in the list
  2. Not so many read/write operations
  3. A LOT of concurrent access (i.e. many threads that access the list in reading mode)

Also can be useful when you need to implement some sort of transaction-like behavior (i.e. revert an insert/update/delete operation in case of fail)

adospace
  • 1,841
  • 1
  • 17
  • 15
1

You can also use the more primitive

Monitor.Enter(lock);
Monitor.Exit(lock);

which lock uses (see this post C# Locking an object that is reassigned in lock block).

If you are expecting exceptions in the code this is not safe but it allows you to do something like the following:

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

public class Something
{
    private readonly object _lock;
    private readonly List<string> _contents;

    public Something()
    {
        _lock = new object();

        _contents = new List<string>();
    }

    public Modifier StartModifying()
    {
        return new Modifier(this);
    }

    public class Modifier : IDisposable
    {
        private readonly Something _thing;

        public Modifier(Something thing)
        {
            _thing = thing;

            Monitor.Enter(Lock);
        }

        public void OneOfLotsOfDifferentOperations(string input)
        {
            DoSomethingWith(input);
        }

        private void DoSomethingWith(string input)
        {
            Contents.Add(input);
        }

        private List<string> Contents
        {
            get { return _thing._contents; }
        }

        private object Lock
        {
            get { return _thing._lock; }
        }

        public void Dispose()
        {
            Monitor.Exit(Lock);
        }
    }
}

public class Caller
{
    public void Use(Something thing)
    {
        using (var modifier = thing.StartModifying())
        {
            modifier.OneOfLotsOfDifferentOperations("A");
            modifier.OneOfLotsOfDifferentOperations("B");

            modifier.OneOfLotsOfDifferentOperations("A");
            modifier.OneOfLotsOfDifferentOperations("A");
            modifier.OneOfLotsOfDifferentOperations("A");
        }
    }
}

One of the nice things about this is you'll get the lock for the duration of the series of operations (rather than locking in each operation). Which means that the output should come out in the right chunks (my usage of this was getting some output onto screen from an external process)

I do really like the simplicity + transparency of the ThreadSafeList + that does the important bit in stopping crashes

Community
  • 1
  • 1
JonnyRaa
  • 7,559
  • 6
  • 45
  • 49
0

I believe _list.ToList() will make you a copy. You can also query it if you need to such as :

_list.Select("query here").ToList(); 

Anyways, msdn says this is indeed a copy and not simply a reference. Oh, and yes, you will need to lock in the set method as the others have pointed out.

MD SHAHIDUL ISLAM
  • 14,325
  • 6
  • 82
  • 89
Jonathan Henson
  • 8,076
  • 3
  • 28
  • 52
-1

Looking at the original sample one may guess that the intention was to be able to simply replace the list with the new one. The setter on the property tells us about it.

The Micrisoft's Thread-Safe Collections are for safely adding and removing items from collection. But if in the application logic you are intending to replace the collection with the new one, one may guess, again, that the adding and deleting functionality of the List is not required.

If this is the case then, the simple answer would be to use IReadOnlyList interface:

 private IReadOnlyList<T> _readOnlyList = new List<T>();

    private IReadOnlyList<T> MyT
    {
       get { return _readOnlyList; }
       set { _readOnlyList = value; }
    }

One doesn't need to use any locking in this situation because there is no way to modify the collection. If in the setter the "_readOnlyList = value;" will be replaced by something more complicated then the lock could be required.

dazipe
  • 11
  • 3
-3

Basically if you want to enumerate safely, you need to use lock.

Please refer to MSDN on this. http://msdn.microsoft.com/en-us/library/6sh2ey19.aspx

Here is part of MSDN that you might be interested:

Public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

A List can support multiple readers concurrently, as long as the collection is not modified. Enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with one or more write accesses, the only way to ensure thread safety is to lock the collection during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

istudy0
  • 1,313
  • 5
  • 14
  • 22
-4

Here is the class for thread safe list without lock

 public class ConcurrentList   
    {
        private long _i = 1;
        private ConcurrentDictionary<long, T> dict = new ConcurrentDictionary<long, T>();  
        public int Count()
        {
            return dict.Count;
        }
         public List<T> ToList()
         {
            return dict.Values.ToList();
         }

        public T this[int i]
        {
            get
            {
                long ii = dict.Keys.ToArray()[i];
                return dict[ii];
            }
        }
        public void Remove(T item)
        {
            T ov;
            var dicItem = dict.Where(c => c.Value.Equals(item)).FirstOrDefault();
            if (dicItem.Key > 0)
            {
                dict.TryRemove(dicItem.Key, out ov);
            }
            this.CheckReset();
        }
        public void RemoveAt(int i)
        {
            long v = dict.Keys.ToArray()[i];
            T ov;
            dict.TryRemove(v, out ov);
            this.CheckReset();
        }
        public void Add(T item)
        {
            dict.TryAdd(_i, item);
            _i++;
        }
        public IEnumerable<T> Where(Func<T, bool> p)
        {
            return dict.Values.Where(p);
        }
        public T FirstOrDefault(Func<T, bool> p)
        {
            return dict.Values.Where(p).FirstOrDefault();
        }
        public bool Any(Func<T, bool> p)
        {
            return dict.Values.Where(p).Count() > 0 ? true : false;
        }
        public void Clear()
        {
            dict.Clear();
        }
        private void CheckReset()
        {
            if (dict.Count == 0)
            {
                this.Reset();
            }
        }
        private void Reset()
        {
            _i = 1;
        }
    }
  • 1
    _i++ is not threadsafe. you have to use an atomic add when you increment it and probably mark it volatile too. CheckReset() is not threadsafe. Anything can happen in between the conditional check and call to Reset(). Don't write your own multithreading utilities. – Chris Rollins Aug 07 '20 at 18:40
-17

Use the lock statement to do this. (Read here for more information.)

private List<T> _list;

private List<T> MyT
{
    get { return _list; }
    set
    {
        //Lock so only one thread can change the value at any given time.
        lock (_list)
        {
            _list = value;
        }
    }
}

FYI this probably isn't exactly what your asking - you likely want to lock farther out in your code but I can't assume that. Have a look at the lock keyword and tailor its use to your specific situation.

If you need to, you could lock in both the get and set block using the _list variable which would make it so a read/write can not occur at the same time.

Josh M.
  • 26,437
  • 24
  • 119
  • 200
  • 2
    That's not going to solve his problem; it only stops threads from setting the reference, not adding to the list. – Tejs May 03 '11 at 19:07
  • And what if one thread is setting the value while another is iterating the collection (it's possible with your code). – Xaqron May 03 '11 at 19:07
  • Like I said, the lock will probably have to be moved out further in the code. This is just an example of how to use the lock statement. – Josh M. May 03 '11 at 21:02
  • Yeah, except it's an example of how **not** to use the lock statement. I can create a complete deadlock and freeze the program with hardly any effort (or by accident) with that example: `var myRef = foo.MyT; lock(myRef) { foo.MyT = myRef; }` – Joel Mueller May 03 '11 at 23:22
  • 2
    @Joel Mueller: Sure, if you manufacturer some silly example like that. I'm just trying to illustrate that the asker should look into the `lock` statement. Using a similar example I could argue that we shouldn't use for loops since you could deadlock the application with hardly any effort: `for (int x = 0; x >=0; x += 0) { /* Infinite loop, oops! */ }` – Josh M. May 04 '11 at 11:39
  • @Joel Mueller: Just to summarize, the code I posted will NOT deadlock the application because the object used in the `lock` statement is being set on the same thread. If you'd like, I can post an example showing this. – Josh M. May 04 '11 at 11:48
  • 6
    I never claimed your code meant instant deadlock. It's a bad answer to this particular question for the following reasons: 1) It doesn't protect against the contents of the list being modified during enumeration of the list, or by two threads at once. 2) Locking the setter but not the getter means the property is not really thread-safe. 3) Locking on **any** reference that is accessible from outside the class is widely considered a bad practice, as it dramatically increases the chances of accidental deadlock. That's why `lock (this)` and `lock (typeof(this))` are big no-no's. – Joel Mueller May 04 '11 at 19:37