• I removed the www from the URL, please update your bookmarks and pay attention to the bottom of the screen if using Google Chrome to allow push notifications again.

an old bug and learning something new

JohnH

VB.NET Forum Moderator
Staff member
Joined
Dec 17, 2005
Messages
15,277
Location
Norway
Programming Experience
10+
I have for some been getting weird exceptions from code to add items to a dictionary like this:
If Not dct.ContainsKey(item.Key) Then
    dct(item.Key) = New List(Of String)
End If

Once every million operations I'm sure I got NullReferenceException before (previous VS version), but could not pinpoint it, neither dictionary, item nor item.Key was Nothing. This was happening in multi-threading with UI ties that maybe wasn't thought through. I ignored it then with a Try-Catch that didn't seem to cause more trouble. Today I got a different exception from same code, the KeyNotFoundException amazingly. I finally figured I should add a synchronization lock and stumbled upon ConcurrentDictionary to handle that. I've never used this class before, it was new with .Net 4. So changed the code to:
dct.TryAdd(item.Key, New List(Of String))

This also made the ContainsKey check redundant. I read that:
All public and protected members of ConcurrentDictionary<TKey,TValue> are thread-safe and may be used concurrently from multiple threads. However, members accessed through one of the interfaces the ConcurrentDictionary<TKey,TValue> implements, including extension methods, are not guaranteed to be thread safe and may need to be synchronized by the caller.
I was wondering if using the default Item property also was thread-safe:
dct(item.Key) = New List(Of String)

According to ConcurrentDictionary<TKey,TValue>.Item[TKey] Property (System.Collections.Concurrent) | Microsoft Docs this implements IDictionary<TKey,TValue>.Item[TKey] Property - so "not guaranteed". Looking at the code in .Net Reflector shows that this code actually is equivalent of the thread-safe AddOrUpdate method. I think the documentation could have mentioned this. Anyway if I wanted to use this code instead, and it should work like before, I would have to add the ContainsKey check again, so TryAdd is better for me in this case. Maybe the standard Dictionary also should get these methods? Reducing 3 lines of code to 1 here and there can do something for readability.

In end I removed a few lines of code, removed a threading bug that happened very rarely, and I think things runs more smoothly now. It's funny though, the weird exceptions that it caused and how difficult it was to figure out. Who would have thought that "dct(key) = value" would throw KeyNotFoundException (or nonsensical NullReferenceException), and that the cause was some kind of threading violation?

The program was written more than 9 years ago, I've been thinking to rewrite it too one day. It's actually a program I use daily and that has worked well as it is.
 
Top Bottom