why is this code causing a memory leak?

I've recently taken ownership of a WCF Windows Service that makes heavy use of the following static Utility class to retrieve lookup data:

   public static class Utility
    {

       //begin code that causes increased memory consumption
        private static Dictionary<string, ErrorData> _errorData;

        internal static Dictionary<string, ErrorData> ErrorData

        {
            get
            {
                if (_errorData == null)
                {
                    _errorData = GetErrorData();
                }
                return _errorData;
            }

        }
        //end code that causes increased memory consumption

        /// GetErrorData method to get error messages from error xml
        /// </summary>         
        /// <returns>Dictionary of Error messages value for different fields.</returns>           
        internal static Dictionary<string, ErrorData> GetErrorData()
        {
            Dictionary<string, ErrorData> data = null;


                XmlDocument doc = LoadXmlDocument(Constants.ErrorMessagesFileName);
                XmlNodeList errorNode = doc.SelectNodes("/ErrorMessages/Error");
                data = new Dictionary<string, ErrorData>();

                foreach (XmlNode node in errorNode)
                {
                    ErrorData errorValues = new ErrorData();
                    errorValues.FieldName = node.Attributes["FieldName"].Value;
                    errorValues.ErrorMessage = node.Attributes["ErrorMessage"].Value;
                    data.Add(node.Attributes["code"].Value, errorValues);
                }


            return data;
        }
        internal static XmlDocument LoadXmlDocument(string xmlFileName)
        {
            XmlDocument doc = null;
            try
            {
                if (HttpRuntime.Cache[xmlFileName] == null)
                {
                    doc = new XmlDocument();
                    doc.Load(Constants.Folderpath + "\\" + xmlFileName);
                    HttpRuntime.Cache.Insert(xmlFileName, doc);
                }
                else
                {
                    doc = (XmlDocument)HttpRuntime.Cache[xmlFileName];
                }
            }
          开发者_开发问答  catch (Exception ex)
            {
               //log
            }
            return doc;
        }
    }

As you can see, the static ErrorData property makes use of a private backing field. ErrorData is a Dictionary that is constructed using an XML resource on the filesystem, which is why the contents of the file are stored in HttpRuntime.Cache upon initial retrieval.

Under normal load, the service consumes about 120 MB of RAM.

At some point, a team member felt the need to introduce another level of optimization by creating a static property backed by a lazily loaded static field. Anyway, the presence of said static field causes a rather severe memory leak (500MB+ ) after just a few calls to the service.

The minute I remove the static field and property (clients instead call Utility.GetErrorData()), memory consumption goes back to normal levels.

Can anyone explain why the presence of this static field is causing a memory leak? The WCF service is running with InstanceContextMode.PerCall, if that makes a difference.

Many thanks.

If the error file is very large, then the static version loads the huge XML document into memory and never releases it. Previously, if clients had called GetErrorData(), then the data would have been loaded into memory and returned, cleaing memory.

There is no synchronization here, so if the static variable were not loaded, several simultaneous requests would have begun loading the error document separately. Only one Dictionary would win and get saved to the static variable. However, if the error file is large, multiple threads loading it simultaneously would increase memory pressure. If this were the case, I would expect the next garbage collection would reclaim the extra instances and release much of this memory.

Also note that the static instance version loads the error file once. So if additional errors were created, these would never be returned to the client.

I'm not entirely sure what code change you mean when you talk about the change. However, from reading the code my guess is that you end up calling GetErrorData more than once, and the dictionary just fills up with lots of duplicate entries. If you add logging code, which code is shown to be entered repeatedly?

Martyn

Try to take dump of your w3p process and check what is there on Large object heap and which objects are occupying large chunk of memory. that will help you reach exact cause of memory leak.

you should also check the assemblies loaded into memory. XMLserializers is one of the examples.

Refer to Tess's blog on memory leak. http://blogs.msdn.com/b/tess/archive/2008/03/17/net-debugging-demos-lab-6-memory-leak.aspx

Does making adding synchronization fix your "memory leak"?

That is, for instance make LoadXmlDocument() and GetErrorData() both private and modify the ErrorData property something like this

    private static Dictionary<string, ErrorData> _errorData;
    private static object lockObject = new object();

    internal static Dictionary<string, ErrorData> ErrorData
    {
        get
        {
            lock (lockObject)
            {
                if (_errorData == null)
                {
                    _errorData = GetErrorData();
                }
                return _errorData;
            }
        }

    }

Note: Usually, a memory leak means that the application slowly over time consume more and more memory (which is never reclaimed). Is this what you are observing, or does your memory consumption just become higher though stable when you change the implementation? To really verify that you indeed have a memory leak and what the real cause is (what objects can't be collected/finalized), you'll often have to use a memory profiler.