Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Unsafe threading with dictionary - Let's break some stuff

So at the bottom of http://msdn.microsoft.com/en-us/library/xfhwa508.aspx we see

A Dictionary(Of TKey, TValue) can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

In my asp.net application, I occasionally see NullReferenceExceptions generated from inside System.Collections.Generic.Dictionary`2.Insert.

How could I recreate this in a sandbox?

The following code doesn't cause the exception, in spite of a local dictionary being accessed by separate threads.

class Program {
    static void Main(string[] args) {
        int maxThreads = 20;
        Dictionary<Guid, Guid> dict = new Dictionary<Guid, Guid>();
        var tws = new threadWithState();
        while (true) {
            for (int i = 0; i < maxThreads * 2; i++) {
                var thread = new Thread(tws.Work);
                thread.IsBackground = true;
                thread.Start(dict);
            }
            dict[Guid.NewGuid()] = Guid.NewGuid();
        }
    }
}

class threadWithState {
    public void Work(object dict) {
        Console.WriteLine((dict as Dictionary<Guid, Guid>).Count);
        for (int i = 0; i < 1000; i++) {
            (dict as Dictionary<Guid, Guid>)[Guid.NewGuid()] = Guid.NewGuid();
        }
    }
}
like image 216
MushinNoShin Avatar asked Dec 16 '22 05:12

MushinNoShin


2 Answers

That's part of the problem with threading - bugs are not necessarily reproducible reliably. The issue is that the insert has to hit a specific race condition, which obviously isn't happening in your test. Of course, enough runs may cause this to trigger, but that's going to be difficult to reliably test.

That being said, this is obviously a problem. Instead of focusing on reproducing it, you should consider how to just correct it. The obvious answer here, of course, is to switch to a different technique, such as using a ConcurrentDictionary<T,U> instead, which is thread safe.

like image 96
Reed Copsey Avatar answered Dec 18 '22 20:12

Reed Copsey


This code suffers from accidental synchronization due to the garbage collected heap lock. You'll get it to crash much quicker by injecting random delays, much like what happens on the actual server. This code made it crash in a fraction of a second on my machine, ymmv:

        public void Work(object dict) {
            var rnd = new Random();
            Console.WriteLine((dict as Dictionary<Guid, Guid>).Count);
            for (int i = 0; i < 1000000; i++) {
                (dict as Dictionary<Guid, Guid>)[Guid.NewGuid()] = Guid.NewGuid();
                for (int ix = 0; ix < rnd.Next(1000); ++ix) ;   // NOTE: random delay
            }
        }
like image 40
Hans Passant Avatar answered Dec 18 '22 18:12

Hans Passant