4

I have a ConcurrentDictionary like this:

ConcurrentDictionary<int, Dto> concurrentDictionary = new ConcurrentDictionary<int, Dto>();

This is readable and writable dictionary that can be used by many threads. I can manage writable side in a thread safe way. When I need to access Dto's in the dictionary, I always use Linq select method to get the Dto's from.

IEnumerable<Dto> dtos = concurrentDictionary.Select(p => p.Value);

I know, Linq methods on concurrent dictionary are lockless and thread safe for a readable and writable concurrent dictionary. After I access to Dto's using Linq, is it thread safe using some read functions on these Dto's? I mean accessing these Dto's with in a foreach loop or making new lists from them and making some filtering or ordering on these Dto's properties?

ConcurrentDictionary<int, Dto> concurrentDictionary = new ConcurrentDictionary<int, Dto>();
for (int i = 0; i < 10000; i++)
{
    concurrentDictionary.TryAdd(i, new Dto()
    { Id = i, Name = RandomString(35), Type = "new", IsActive = true} );
}

IEnumerable<Dto> dtos = concurrentDictionary.Select(p => p.Value);

ArrayList arrayList = new ArrayList();
foreach (object obj in dtos)
{
    //is it thread safe accessing the Dto like this and adding them to new arraylist?
    arrayList.Add(obj);
}

//Is it thread safe accessing the Dto's properties like this? 
//Of course only for reading purposes.
string name = ((Dto) arrayList[0]).Name;
rodins
  • 306
  • 2
  • 11
  • 1
    Are the DTOs immutable? If so, then no problem. If not, then you have no way of knowing if some code can mutate them. A thread could add a DTO to the dictionary and keep a reference to the DTO, which it then uses later on to modify the DTO's properties. – Matthew Watson Jan 08 '18 at 11:13
  • Nothing to do with the actual question, but don't use ArrayList when you can use List. – Alex Paven Jan 08 '18 at 11:38
  • Dtos are not immutable. Dtos can be added to concurrent dictionary or can be update from any thread. In this situation, Can I create a new list for each thread and use this lists from each thread for reading purposes? IList dtos2= concurrentDictionary.Select(p => p.Value).ToList(); This will create a new List but Dto's will not be copied to new created list, they will only passed by reference. Are this new created lists will be thread safe (of course for reading like filtering)? – rodins Jan 08 '18 at 11:41
  • 1
    The term "thread-safe" simply means that your code will [function correctly in a multi-threaded environment](https://en.wikipedia.org/wiki/Thread_safety), but what is _correct_ can only be defined by you. "Can you create a new list for each thread and use it for reading purposes?" Yes. Can this list be affected by changes made on other threads? Yes. If you want a list that is unaffected by changes on other threads, then make your `Dto`s immutable. If you can't do that, then you will need to clone the `Dto`s you select from the concurrent dictionary. – bornfromanegg Jan 08 '18 at 14:24
  • @bornfromanegg I understand what you say. Here, when I say "thread-safe", I mean **"the code will function correctly in a multi-threaded environment"**. So I ask my question in this manner. Yes. This list can be affected by changes made on other threads. This is not a problem. I only confused about **will code work correctly without giving any error**. I also find a good test about concurrent dictionary and thread-safety. Here is the [link](http://jeremyrsellars.github.io/no-new-legacy/posts/2016-10-21-concurrentdictionary-and-the-pit-of-success/) – rodins Jan 09 '18 at 09:59

1 Answers1

5

According to the documentation on GetEnumerator for ConcurrentDictionary (the highlighting is mine):

The enumerator returned from the dictionary is safe to use concurrently with reads and writes to the dictionary, however it does not represent a moment-in-time snapshot of the dictionary. The contents exposed through the enumerator may contain modifications made to the dictionary after GetEnumerator was called.

Whether or not you consider this threadsafe depends on exactly what semantics you require. Note that if you do want a snapshot, you can get this by calling the ToArray() method:

IEnumerable<Dto> dtos = concurrentDictionary.ToArray().Select(p => p.Value);

According to the documentation, ToArray()returns:

A new array containing a snapshot of key and value pairs copied from the System.Collections.Concurrent.ConcurrentDictionary.

(Note that this is the ToArray method belonging to ConcurrentDictionary, NOT the Linq ToArray() method, which will use the enumerator.)

Finally, note that, as has already been pointed out, if your DTOs are mutable, all bets are off. You may get a snapshot of the list, but there is nothing preventing the contents of that list being changed on another thread.

EDIT:

From reading the page that you linked to in the comments, the takeaway point is this:

All public and protected members of ConcurrentDictionary are thread-safe and may be used concurrently from multiple threads. However, members accessed through one of the interfaces the ConcurrentDictionary implements, including extension methods, are not guaranteed to be thread safe and may need to be synchronized by the caller.

This tells us that Linq is not guaranteed to be threadsafe when used with ConcurrentDictionary. for each should be fine, since that will call GetEnumerator which has the guarantee mentioned at the beginning of this post. My hunch is that Select would also be fine, but it is not guaranteed to be.

So to answer the questions in your code:

IEnumerable<Dto> dtos = concurrentDictionary.Select(p => p.Value);

ArrayList arrayList = new ArrayList();
foreach (object obj in dtos)
{
    //is it thread safe accessing the Dto like this and adding them to new arraylist?
    arrayList.Add(obj);
}

Probably, but it is not guaranteed, and in any case, this gives you no benefit over ToArray(), so I would replace everything here with:

KeyValuePair<int, Dto>[] dtoArray = concurrentDictionary.ToArray();

Secondly:

//Is it thread safe accessing the Dto's properties like this? 
//Of course only for reading purposes.
string name = ((Dto) arrayList[0]).Name;

Whether this is safe or not has nothing to do with ConcurrentDictionary and everything to do with your implementation of the Name method in Dto. Yes, you can access the Dto safely, but any properties or methods you access would need to be threadsafe. If you have used ToArray() as suggested above, then

Dto dto = dtoArray[0].Value

is threadsafe, but

string name = dtoArray[0].Value.Name

depends on the implementation of Name.

bornfromanegg
  • 2,826
  • 5
  • 24
  • 40
  • Thank you @bornfromanegg. ConcurrentDictionary's ToArray() also is the recommended way by others. This is discussed also here: https://stackoverflow.com/questions/11692389/getting-argument-exception-in-concurrent-dictionary-when-sorting-and-displaying. It causes a performance overhead by locking the dictionary internally but gives an errorless functionality. A good and useful test is here: http://jeremyrsellars.github.io/no-new-legacy/posts/2016-10-21-concurrentdictionary-and-the-pit-of-success/ – rodins Jan 09 '18 at 10:16
  • 1
    I've edited the answer. Note also that if you are worried about performance, you should not be using `ArrayList`. In fact, just don't use `ArrayList` ever - use one of the [generic collection types](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/collections), or use an array. – bornfromanegg Jan 09 '18 at 11:27
  • Thank you. I always use generic collections instead of ArrayList. Example using ArrayList is from some other codes. As I undertand, after ToArray() method which make a snapshot and creates a new array, I can use Linq safely. So, concurrentDictionary.ToArray().Select(p => p.Value) or concurrentDictionary.ToArray().Select(p => p.Value).ToList() is also safe for errorless code functionality. Is it right? – rodins Jan 09 '18 at 12:57