Lock entire Dictionary or part of before copying (SyncRoot)

Question

I have 2 dictionaries in a scoped service within a Blazor server application I use to manage state for multi tenancy. It has come to my attention that there may be concurrency issues with users modifying the dictionaries on different threads.

public Dictionary<string, GlobalDataModel> Global { get; } = new();
public Dictionary<string, Dictionary<long, LocalDataModel>> Local { get; } = new();

I think I'm leaning towards leaving these as normal dictionaries and locking sync root when modifying or iterating over.

If I were to add an item to a collection within the containing class, would it be:

if (Local.ContainsKey(tenantId) == false)
{
    lock (syncRoot)
    {
        Local.Add(tenantId, new Dictionary<long, LocalDataModel>());
    }
}

or:

lock (syncRoot)
{
    if (Local.ContainsKey(tenantId) == false)
    {
        {
            Local.Add(tenantId, new Dictionary<long, LocalDataModel>());
        }
    }
}

Then if I were to copy different parts of the service collection to lists from an external class so it can be safely iterated would I just lock at the Service level, the Dictionary level, or the DataModel level, or is it dependent?

Say I wanted a resources collection within a specific project. Would it be:

[Inject] private IDataAdaptor DataAdaptor { get; set; };
var resources;
    
lock (DataAdaptor)
{
    resources = DataAdaptor.Local[TenantId][ProjectId].Resources;
}

or:

lock (DataAdaptor.Local[TenantId][ProjectId].Resources)
{
   resources = DataAdaptor.Local[TenantId][ProjectId].Resources;
}

And vice versa for different parts of the collection, et Tenants, Projects etc...

I assume I have to lock on the object because syncRoot isn't accessible outside the containing class, or do I create a SyncRoot object in the class where I'm copying and lock on that?

Obviously multi threading is a new concept.

I looked at concurrent dictionary and in this case i will still have the same issue that it could be updated while someone is iterating and also had a problem prefilling on load for reasons too involved for this post — Pippo, Jan 15 '23 at 07:58
Syncroot is an object in my 'DataAdaptor' service/class... public object syncRoot = new(); — Pippo, Jan 15 '23 at 08:00
I may have inadvertently called my service a DataAdapter though it does not use the System.Data.Common though it does have similarities — Pippo, Jan 15 '23 at 08:02
If reads are often but writes are rare - consider also using `ReaderWriterLockSlim`. Then you can have multiple simultaneous readers while there are no writes. — Evk, Jan 15 '23 at 08:20

Theodor Zoulias · Accepted Answer · 2023-01-15T06:33:39.260

6

The Dictionary<TKey,TValue> is thread-safe only when used by multiple readers and zero writers. As soon as you add a writer in the mix, in other words if the dictionary is not frozen and can be mutated, then it is thread-safe for nothing. Every access to the dictionary, even the slightest touch like reading its Count, should be synchronized, otherwise its behavior is undefined.

Synchronizing a dictionary can be quite inefficient, especially if you have to enumerate it and do something with each of its entries. In this case you have to either create a copy of the dictionary (.ToArray()) and enumerate the copy, or lock for the whole duration of the iteration, blocking all other threads that might want to do trivial reads or writes. Alternative collections that are better suited for multithreaded environments are the ConcurrentDictionary<TKey,TValue> and the ImmutableDictionary<TKey,TValue>.

In case you want to educate yourself systematically about thread-safety, here is an online resource: Threading in C# - Thread Safety by Joseph Albahari.

edited Jan 15 '23 at 06:33

answered Jan 15 '23 at 06:10

Theodor Zoulias

34,835
7
69
104

Thank you for the reply, i had looked at ConcurrentDictionary, however on testing it seemed there was still the chance that it could change while another thread was iterating? I also had an issue filling it on the initial load. maybe i will go revisit it. I was aiming to copy, i only need certain parts at any one time so can copy to list easy enough and then iterate etc. And thankyou for the link to thread safety I will read in detail. – Pippo Jan 15 '23 at 08:05
@Pippo I don't know how you used the `ConcurrentDictionary`, but keep in mind that its thread-safety is limited to itself, and it is not extended to the values it contains. So if for example the `TValue` is `List`, and the lists are changed by multiple threads concurrently, you'll have to synchronize each list independently. – Theodor Zoulias Jan 15 '23 at 11:20
So when updating a TValue as a list do you lock on the list that is being updated or the dictionary – Pippo Jan 15 '23 at 21:46
@Pippo you lock on the list, to minimize the contention. This way other threads will not be blocked, unless they try to mutate the same list. Btw be aware that the `ConcurrentDictionary` is not an easy class to handle. It has a number of non-obvious traps to fall in, and everyone one is falling in them until they master the class. – Theodor Zoulias Jan 15 '23 at 22:34
Thanks, probably why I couldn’t get it working how I wanted with my initial attempt – Pippo Jan 15 '23 at 23:22
So locking on a syncRoot object prevents the same block of code being run in deferent threads. But if I lock on List that will prevent mutation of the list until the lock is released even if from another thread in the same class or a different thread and different block of code in a different class? Ie not only locking access to a block of code that modifies the list but also locking the list so it can’t be edited? Conceptualising this is proving difficult and I need it spelled out apparently. – Pippo Jan 16 '23 at 07:16
@Pippo the thread-safety of the `List` is similar to the thread-safety of the `Dictionary`. If it's used by readers and writers concurrently, it must be synchronized meticulously everywhere, otherwise its behavior is undefined. Synchronized means that only one thread at a time should be allowed to interact with the list. – Theodor Zoulias Jan 16 '23 at 08:03
Will lock(List) {…} prevent that list being mutated by another thread while the subsequent code block runs? – Pippo Jan 16 '23 at 08:31
@Pippo if the other thread is mutating the list without first entering a `lock`-protected region using the same object as locker, then no, the `lock(List)` on the current thread won't prevent the other thread from mutating the list. Locking is cooperative. It cannot be enforced from one thread to another, in a non-cooperative manner. – Theodor Zoulias Jan 16 '23 at 08:36
1

Gottya just use same object as locker. Thanks for patience explaining. – Pippo Jan 16 '23 at 08:41

Lock entire Dictionary or part of before copying (SyncRoot)

1 Answers1