• Hello and welcome to our new forums. We upgraded our forum sites to a more robust and modern system which we hope you will enjoy. Be sure to check out your profile by clicking the button on the top right and configure your preferences, signature, time zone, avatar, etc. as you wish. If you need help with using this new forum'ware try the help link on the bottom right.

    Click here to review your account now.

an old bug and learning something new

JohnH

VB.NET Forum Moderator
Staff member
Joined
Dec 17, 2005
Messages
15,254
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