Multithreaded access to a Dictionary

Question

I've search across the web and I get a bit confused about multithreading (lock, Monitor.Enter, volatile etc.) So, instead of asking the solution here, I've tried something "homemade" about multithreading management and I would like to have your advices.

Here's my context :

-I have a static class that contains a static Dictionary<int,string>

-I have a lot of tasks (let's say 1000) reading in this Dictionary every seconds

-I have one another task, that will update this Dictionary every 10s.

Here's the code of the cache:

public static class Cache
{
    public static bool locked = false;
    public static Dictionary<int, string> Entries = new Dictionary<int, string>();
    public static Dictionary<int, string> TempEntries = new Dictionary<int, string>();

    // Called by 1000+ Tasks
    public static string GetStringByTaskId(int taskId)
    {
        string result;

        if (locked)
            TempEntries.TryGetValue(taskId, out result);
        else
            Entries.TryGetValue(taskId, out result);

        return result;
    }

    // Called by 1 task
    public static void UpdateEntries(List<int> taskIds)
    {
        TempEntries = new Dictionary<int, string>(Entries);

        locked = true;
        Entries.Clear();

        try
        {
            // Simulates database access
            Thread.Sleep(3000);

            foreach (int taskId in taskIds)
            {
                Entries.Add(taskId, $"task {taskId} : {DateTime.Now}");
            }
        }
        catch (Exception ex)
        {
            Log(ex);
        }
        finally
        {
            locked = false;
        }
    }
}

There's my questions :

The program runs but I don't understand why the twice 'locked' bool assignments in the UpdateEntries methods doesn't generate a multithreading exception since it's read by the other thread "everytime"

Is there a more conventional way to handle this, I feel like it's a weird way to do it?

"I get a bit confused about Multithreading (lock, Monitor.Enter, volatile etc.)" but only they are the right ways to handle multithreading challenges. Your code above won't work at all. — Lex Li, Mar 09 '21 at 23:35
@LexLi In fact it works lol And thats what I want to understand : why does it works ? — 善逸様, Mar 09 '21 at 23:48
If you have time to study multithreading systematically, here is a great online resource: [Threading in C#](http://www.albahari.com/threading/) by Joseph Albahari. In order to write correct multithreaded code, some solid understanding of the basics is required. — Theodor Zoulias, Mar 10 '21 at 00:05

John Wu · Accepted Answer · 2021-03-09T23:32:40.887

The conventional way to handle this is to use a ConcurrentDictionary. This class is thread-safe and designed for multiple threads to read and write to it. You still need to be aware of potential logic problems (e.g. if two keys must be added at the same time before other threads can see either of them), but it is going to be fine for most operations without additional locking.

Another way to handle this for your specific situation is to use an ordinary dictionary, but treat it as immutable once it is available to reader threads. This will be more efficient as it avoids locks.

public static void UpdateEntries(List<int> taskIds)
{
    //Other threads can't see this dictionary
    var transientDictionary = new Dictionary<int, string>();  

    foreach (int taskId in taskIds)
    {
        transientDictionary.Add(taskId, $"task {taskId} : {DateTime.Now}");
    }

    //Publish the new dictionary so other threads can see it
    TempEntries = transientDictionary; 
}

Once the dictionary is assigned to TempEntries (the only place other threads can access it), it is never modified, so the threading concerns go away.

Based on "Here's the code of the cache:" it may be better to use `MemoryCache` is OP really needs proper cache with thread safety and expirations. — Alexei Levenkov, Mar 09 '21 at 23:55
Your `transientDictionary` isn't constructing from the `Entries` dictionary so it will lose all the stuff that was already in it right? Not sure how this can add to existing data. — Michael Dorgan, Mar 09 '21 at 23:55
[Is a reference assignment thread-safe?](https://stackoverflow.com/a/5209632/2791540) — John Wu, Mar 09 '21 at 23:59

score 1 · Answer 2 · answered Mar 10 '21 at 15:11

Using a non-volatile bool flag for thread synchronization is not thread-safe, and makes your code susceptible to race conditions and haisenbugs. The correct way to do it is to replace atomically the old dictionary with the new one, after the new one has been fully constructed, using a cross-thread publishing mechanism like the Volatile.Write or the Interlocked.Exchange methods. Your case is simple enough that you could also use the volatile keyword for brevity, like in the example below:

public static class Cache
{
    private static volatile ReadOnlyDictionary<int, string> _entries
        = new ReadOnlyDictionary<int, string>(new Dictionary<int, string>());

    public static IReadOnlyDictionary<int, string> Entries => _entries;

    // Called by 1000+ Tasks
    public static string GetStringByTaskId(int taskId)
    {
        _entries.TryGetValue(taskId, out var result);
        return result;
    }

    // Called by 1 task
    public static void UpdateEntries(List<int> taskIds)
    {
        Thread.Sleep(3000); // Simulate database access

        var temp = new Dictionary<int, string>();
        foreach (int taskId in taskIds)
        {
            temp.Add(taskId, $"task {taskId} : {DateTime.Now}");
        }
        _entries = new ReadOnlyDictionary<int, string>(temp);
    }
}

With this approach every access to the _entries field will incur the cost of volatility, which is typically less than 10 nsec per operation, so it shouldn't be a problem. It's a cost worth paying, because it guarantees the correctness of your program.

Multithreaded access to a Dictionary

2 Answers2