Alex Rivera | Logout

Exception when adding Dictionary entry

Asked 2011-11-16T11:49:36.730
8

We're seeing this exception occur in the following block of code in an ASP.NET context which is running on an IIS 7 server.

1) Exception Information
*********************************************  
Exception Type: System.Exception  
Message: Exception Caught in Application_Error event
Error in: InitializationStatus.aspx  
Error Message:An item with the same key has already been added.  
Stack Trace:    at
System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)   
at CredentialsSession.GetXmlSerializer(Type serializerType)

This is the code that the exception is occuring in:

[Serializable()]
public class CredentialsSession
{
    private static Dictionary<string, System.Xml.Serialization.XmlSerializer> localSerializers = new Dictionary<string, XmlSerializer>();

    private System.Xml.Serialization.XmlSerializer GetXmlSerializer(Type serializerType)
    {
        string sessionObjectName = serializerType.ToString() + ".Serializer";

        if (Monitor.TryEnter(this))
        {
            try
            {
                if (!localSerializers.ContainsKey(sessionObjectName))
                {
                    localSerializers.Add(sessionObjectName, CreateSerializer(serializerType));
                }
            }
            finally
            {
                Monitor.Exit(this);
            }
        }
        return localSerializers[sessionObjectName];
    }

    private System.Xml.Serialization.XmlSerializer CreateSerializer(Type serializerType)
    {
        XmlAttributes xmlAttributes = GetXmlOverrides();

        XmlAttributeOverrides xmlOverrides = new XmlAttributeOverrides();
        xmlOverrides.Add(typeof(ElementBase), "Elements", xmlAttributes);

        System.Xml.Serialization.XmlSerializer serializer =
            new System.Xml.Serialization.XmlSerializer(serializerType, xmlOverrides);

        return serializer;
    }
}

The Monito

c# .net multithreading .net-4.0 dictionary
Edit
Report

1 Answer

5

Your code is not thread-safe.

  1. You're locking on this, a CredentialsSession instance, but accessing a static dictionary which can be shared by multiple CredentialsSession instances. This explains why you're getting the error - two different CredentialsSession instances are attempting to write to the dictionary concurrently.

  2. Even if you change this to lock on a static field as suggested in @sll's answer, you aren't thread-safe, because you aren't locking when reading the dictionary. You need a ReaderWriterLock or ReaderWriterLockSlim to efficiently allow multiple readers and a single writer.

    Therefore you should probably use a thread-safe dictionary. ConcurrentDictionary as others have said if you're using .NET 4.0. If not you should implement your own, or use an existing implementation such as http://devplanet.com/blogs/brianr/archive/2008/09/26/thread-safe-dictionary-in-net.aspx.

Your comments suggest you want to avoid calling CreateSerializer for the same type multiple times. I don't know why, because the performance benefit is likely to be negligible, since contention is likely to be rare and can't exceed once for each type during the lifetime of the application.

But if you really want this, you can do it as follows:

var value;
if (!dictionary.TryGetValue(key, out value))
{
    lock(dictionary)
    {
        if(!dictionary.TryGetValue(key, out value))
        {
            value = CreateSerializer(...);
            dictionary[key] = value;
        }
    }
}

From comment:

if I implement this with ConcurrentDictionary and simply call TryAdd(sessionObjectName, CreateSerializer(serializerType)) every time.

answered 2011-11-16T12:32:32.547

Your Answer