ConcurrentDictionary AddOrUpdate error

Question

I stumbled upon the following obstacle. I need to have a ConcurrentDictionary<string, ArrayList> which has string as key and ArrayList as value. I want to use the AddOrUpdate in the following manner:

using System.Collections;
using System.Collections.Concurrent;

private ConcurrentDictionary<string, ArrayList> _data= new ConcurrentDictionary<string, ArrayList>();

private void AddData(string key, string message){
     _data.AddOrUpdate(key, new ArrayList() { message }, (string existingKey, string existingList) => existingList.Add(message));

}

However this method does not work and throws the following error:

Compiler Error CS1661: Cannot convert anonymous method block to delegate type 'delegate type' because the specified block's parameter types do not match the delegate parameter types

See link to error

In conclusion, I am trying to do the following:

Try adding message to arraylist in the ConcurrentDictionary.
If arraylist DOES NOT exist, create a new one with the message in it.
If arraylist DOES exist, simply add it to the end of the array.

So my question is, how can I fix this error and improve my code, what am I doing wrong ?

your lambda should *return* new value for key, not just modify existing value inplace. Moreover, even if you will return modified value, I'm afraid that such a modifying is not thread-safe — Serg, Oct 14 '21 at 09:26
Microsoft recommends to use List instead: https://learn.microsoft.com/en-us/dotnet/api/system.collections.arraylist?view=net-5.0 — Patrick, Oct 14 '21 at 09:28
Please don't use fairly complex collections such as `ConcurrentDictionary` unless you have a good grasp at writing multi-threaded code already, and you understand the pitfalls. You're not thinking about the fact that any modifications to the ArrayList need to be thread-safe as well, for example. You're probably going to be much better off using a simple lock around a `Dictionary` — canton7, Oct 14 '21 at 09:31
The idea is that I am making a sample mechanism, in this mechanism im keeping counts in specified keys. Like "firstKey" has value: "3" . every time when this reaches some boundary, the message that it comes with, will be sampled. I haven't included this in the questoin as it's not important. However I am sure that I have achieved thread safety because the counts are also kept in a concurrentdictionary. — schweppes0x, Oct 14 '21 at 09:35
The factory parameters of a concurrent collection are safe from the perspective the internal consistency of the collection however they can run more than once when there is contention . There is no internal lock purse — TheGeneral, Oct 14 '21 at 09:42
schweppes0x the "update" in the name `AddOrUpdate` means replace the existing value with another one, not modify the existing value. The `ConcurrentDictionary` offers no protection against concurrent mutations by multiple threads for the values it contains, in case these are mutable objects. — Theodor Zoulias, Oct 14 '21 at 09:51
What method would you guys suggest in this situation? how would i keep an count otherwise of these unique keys ? Every one of these keys can have multiple collected messages and this increases as the count goes up. — schweppes0x, Oct 14 '21 at 11:28
@schweppes0x I recommend what I said above -- use a simple `Dictionary>` and a separate lock. Acquire the lock before querying or updating the dictionary — canton7, Oct 14 '21 at 13:22
How would I be sure that no lock contention would occur because there will be flying at least 400.000 messages a day through the application. @canton7 — schweppes0x, Oct 14 '21 at 13:51
You can't be sure there's no lock contention -- that's the point of a lock. But 400,000/day is about 4.5/second, which is nothing. The stuff inside the lock should take single-digit microseconds, so contention is very unlikely — canton7, Oct 14 '21 at 13:56
Note that ConcurrentDictionary doesn't avoid lock contention either, and in fact it behaves *worse* than a simple Dictionary+lock for many usage patterns — canton7, Oct 14 '21 at 13:57
Thank you very much! I will try to do it with the Dictionary+lock method! — schweppes0x, Oct 15 '21 at 06:30
I am not sure why the ConcurrentDictionary is worse than a lock+Dictionary, would you like to explain or direct me towards some kind of source ? I even found some sources saying that the performance of ConcurrentDictionary are far better than the lock+dict — schweppes0x, Oct 15 '21 at 07:23
@schweppes0x IIRC, ConcurrentDictionary is optimized for heavy contention -- a lot of effort goes into fine-grained locking, for example. The worst-case behaviour of ConcurrentDictionary is pretty good, but it comes at the cost of less-than-ideal behaviour in scenarios with less contention. — canton7, Oct 15 '21 at 08:48
@schweppes0x - Your code isn't throwing an error; it's unable to compile according to the error you've linked to. Can you confirm you have the right error? — Enigmativity, Oct 16 '21 at 02:09
schweppes0x you could check out this question: [ConcurrentDictionary with multiple values per key, removing empty entries](https://stackoverflow.com/questions/60695167/concurrentdictionary-with-multiple-values-per-key-removing-empty-entries). If you don't have the requirement of removing empty lists from the dictionary, then the answer to your problem is in the body of that question. — Theodor Zoulias, Oct 16 '21 at 07:06

score 1 · Accepted Answer · answered Oct 26 '21 at 09:14

The correct thread-safe way is:

using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;

private ConcurrentDictionary<string, List<string>> _data = new ConcurrentDictionary<string, List<string>();

private void AddData(string key, string message){
     var list = _data.GetOrAdd(key, _ => new List<string>());

     lock(list) 
     {
        list.Add(message);
     } 

}

ConcurrentDictionary AddOrUpdate error

1 Answers1