Search code examples
.netdictionarythread-safetyajaxcontroltoolkit

Is a Dictionary<K,V> thread safe for simulataneous reading and additions?


This question relates to a very specific and common scenario in which a dictionary is being used for on-demand caching of items in a multi-threaded environment. To avoid thread locking it's preferable to test for an existing cache item outside of a sync lock, but if we subsequently have to add an item then that counts as a write to the dictionary and therefore most advice I've been reading on stackoverflow is that you need to lock on the read as well as the write because the dictionary's internal state might be being altered by the calls to add().

However, looking through Microsoft's AjaxControlToolkit (scriptObjectBuilder class) the code does in fact perform TryGet() outside of any locks and only locks to Add() new items to the dictionary. I can see how this might be possible if the bucket that an item is placed into never changes once added, but my suspicion is that this is wrong and might be a source of bugs.

Thanks.

UPDATE Going by the .Net documentation I think that the described pattern is indeed wrong. However I was wondering if the particular implementation of Dictionary allows it and that the AjaxControlToolkit was relying on this (which would be dubious). On examining the code in Reflector I'm pretty sure that this is indeed wrong, the Dictionary.Resize() method reallocates the number of buckets and moves bucket items around, so any thread in the middle of a TryGet() could potentially be working on unstable data.

UPDATE A defect was already logged against the AjaxControlToolkit on codeplex. See:


Solution

  • To answer my own question following some investingation: On examining the code for Dictionary I can see that the Dictionary.Resize() method reallocates the number of internal buckets that are used for storing data, and redistributes the bucket contents so that items are in the correct bucket based on their hashcode. As such any thread in the middle of a TryGet() risks working on unstable data.

    As an aside, a possible low-locking approach for a Dictionary class might be to place a lock arounf just the Resize() method.