views:

447

answers:

5

In the following code:

public class StringCache
{
    private readonly object lockobj = new object();

    private readonly Dictionary<int, string> cache = new Dictionary<int, string>();

    public string GetMemberInfo(int key)
    {
        if (cache.ContainsKey(key))
            return cache[key];
        lock (lockobj)
        {
            if (!cache.ContainsKey(key))
                cache[key] = GetString(key);
        }
        return cache[key];
    }

    private static string GetString(int key)
    {
        return "Not Important";
    }
}

1) Is ContainsKey thread safe? IOW, what happens if that method is executing when another thread is adding something to the dictionary? 2) For the first return cache[key], is there any chance that it could return a garbled value?

TIA,

MB

+6  A: 

The inherent thread safety of ContainsKey doesn't matter, since there is no synchronization between ContainsKey & cache[key].

For example:

if (cache.ContainsKey(key))
   // Switch to another thread, which deletes the key.
   return cache[key];

MSDN is pretty clear on this point:

To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

For more info, JaredPar posted a great blog entry at http://blogs.msdn.com/jaredpar/archive/2009/02/11/why-are-thread-safe-collections-so-hard.aspx on thread-safe collections.

Michael
+1  A: 

Here's what it says in the MSDN documentation:

Public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

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.

If I'm reading that correctly, I don't believe that it is thread safe.

BFree
+2  A: 

No, ContainsKey is not thread-safe if you're writing values while you're trying to read.

Yes, there is a chance you could get back invalid results -- but you'll probably start seeing exceptions first.

Take a look at the ReaderWriterLockSlim for locking in situations like this -- it's built to do this kind of stuff.

jsw
+1  A: 

Dictionary is not Thread-Safe.

If you say that

what happens if that method is executing when another thread is adding something to the dictionary?

then I suppose other functions access the cache as well. You need to synchronize accesses(reading and writing) to the cache. Use your lock object in all of these operations.

bruno conde
+1  A: 

I believe its not thread safe,

I would suggest go thru below link, it shows implementation of the thread safe dictionary, or its better to develop your own synchronization.

http://lysaghtn.weebly.com/synchronised-dictionary.html

Mutant