Static methods updating a Dictionary<T,U> in ASP.NET - is it safe to lock() on the dictionary itself?

I have a class that maintains a static dictionary of cached lookup results from my domain controller - users' given names and e-mails.

My code looks something like:

private static Dictionary<string, string> emailCache = new Dictionary<string, string>();

protected string GetUserEmail(string accountName)
{
    if (emailCache.ContainsKey(accountName))
    {
        return(emailCache[accountName]);
    }

    lock(/* something */)
    {
        if (emailCache.ContainsKey(accountName))
        {
            return(emailCache[accountName]);
        }

        var email = GetEmailFromActiveDirectory(accountName);
        emailCache.Add(accountName, email);
        return(email);
    }
}

Is the lock required? I assume so since multiple requests could be performing lookups simultaneously and end up trying to insert the same key into the same static dictionary.

If the lock is required, do I need to create a dedicated static object instance to use as the lock t开发者_StackOverflow中文版oken, or is it safe to use the actual dictionary instance as the lock token?

Collections in .NET are not thread safe so the lock is indeed required. An alternative to using the dictionary one could use Concurrent dictionaries introduced in .NET 4.0

http://msdn.microsoft.com/en-us/library/dd287191.aspx

Yes, the lock is required as long as code on other threads can/will access the static object.

Yes, its safe to lock on the dictionary itself, as long as its not accessible via a public getter. Then the caller might use the object for locking itself and that might result in deadlocks. So i would recommend to use a separate object to lock in if your dictionary is somewhat public.

The lock is indeed required.

By using lock, you ensure that only one thread can access the critical section at one time, so an additional static object is not needed.

You can lock on the dictionary object itself, but I would simply use a object lock =new object(); as my lock.

MSDN documentation specify that you should never use the lock() statement over a public object that can be read or modified outside your own code.

I would rather use an object instance rather than the object you attempt to modify, specifically if this dictionnary has accessors that allows external code to access it.

I might be wrong here, I didn't write a line of C# since one year ago.

Since the dictionary is private, you should be safe to lock on it. The danger with locking (that I'm aware of) is that other code that you're not considering now could also lock on the object and potentially lead to a deadlock. With a private dictionary, this isn't an issue.

Frankly, I think you could eliminate the lock by just changing your code to not call the dictionary Add method, instead using the property set statement. Then I don't believe the lock at all.

UPDATE: The following is a block of code from the private Insert method on Dictionary, which is called by both the Item setter and the Add method. Note that when called from the item setter, the "add" variable is set to false and when called from the Add method, the "add" variable is set to true:

if (add)
{
    ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_AddingDuplicate);
}

So it seems to me that if you're not concerned about overwriting values in your dictionary (which you wouldn't be in this case) then using the property setter without locking should be sufficient.

As far as I could see, additional object as a mutex was used:

private static object mutex = new object();

protected string GetUserEmail(string accountName)
{
    lock (mutex)
    {
        // access the dictionary
    }
}