48

I've been googling for the past few hours and trying different things but can't seem to the bottom of this....

When I run this code, the memory usage continuously grows.

while (true)
{
    try
    {
        foreach (string sym in stringlist)
        {
            StreamReader r = new StreamReader(@"C:\Program Files\" + sym + ".xml");
            XmlSerializer xml = new XmlSerializer(typeof(XMLObj), new XmlRootAttribute("rootNode"));
            XMLObj obj = (XMLObj)xml.Deserialize(r);                       
            obj.Dispose();
            r.Dispose();
            r.Close();
        }
    }    
    catch(Exception ex) 
    {
        Console.WriteLine(ex.ToString()); 
    }
    Thread.Sleep(1000);
    Console.Clear();
}

XMLObj is a custom object

[Serializable()]
public class XMLObj: IDisposable
{
    [XmlElement("block")]
    public List<XMLnode> nodes{ get; set; }

    public XMLObj() { }

    public void Dispose()
    {
        nodes.ForEach(n => n.Dispose());
        nodes= null;

        GC.SuppressFinalize(this);
    }
}

I've tried adding in GC.Collect(); but that doesn't seem to do anything.

John Saunders
  • 160,644
  • 26
  • 247
  • 397
Alex999
  • 502
  • 2
  • 5
  • 8
  • 6
    If you think `GC.Collect` solves "memory leaks", you are looking in the wrong place: http://blogs.msdn.com/b/ricom/archive/2003/12/02/40780.aspx. Just because memory usage goes up, does not mean you have a memory leak. I'd also suggest researching Garbage Collection is general: http://msdn.microsoft.com/en-us/library/0xy59wtx(v=vs.110).aspx – Nathan A May 27 '14 at 19:12
  • XmlNode does not implement IDisposable. If your class has no finalizer youd not need to call GC.SuppressFinalize. Use PerfView (http://www.microsoft.com/en-us/download/details.aspx?id=28567) to check out who holds you memory. I do not think the code you posted has a leak at all. – Alois Kraus May 27 '14 at 19:15
  • This isn't an answer to the question, but consider moving the construction of the XmlSerializer instance outside the loop. This will likely improve performance and reduce memory overhead. – Dan Bryant May 27 '14 at 19:15
  • I've came across this excellent blog post here which explains why the `XmlSerializer` can cause Memory Leaks: http://techknackblogs.com/2012/10/xmlserializer-may-cause-memory-leak/ Also you should make use of the `using () { }` statements which will take care of your resources even in case of an exception. In your code, if an Exception is thrown the resources will not be disposed. – Kai Eichinger May 27 '14 at 19:15
  • @DanBryant Your recommendation, although is accurate from a precompiled point of view, you'd be amazed how advanced compilers are now (I did some testing and found that for some foreach's, defining a variable inside or outside made no differnce in the IL). – Erik Philips May 27 '14 at 19:17
  • @ErikPhilips, it can do a pretty good job, but it's more challenging if you're invoking a constructor, as that could potentially have side effects. The compiler would also need to know that you can call the Deserialize method more than once without it mutating the state of the instance in such a way that subsequent calls could yield different results. – Dan Bryant May 27 '14 at 19:20
  • I have edited your title. Please see, "[Should questions include “tags” in their titles?](http://meta.stackoverflow.com/questions/19190/)", where the consensus is "no, they should not". – John Saunders May 27 '14 at 19:22

6 Answers6

98

The leak is here:

new XmlSerializer(typeof(XMLObj), new XmlRootAttribute("rootNode"))

XmlSerializer uses assembly generation, and assemblies cannot be collected. It does some automatic cache/reuse for the simplest constructor scenarios (new XmlSerializer(Type), etc), but not for this scenario. Consequently, you should cache it manually:

static readonly XmlSerializer mySerializer =
    new XmlSerializer(typeof(XMLObj), new XmlRootAttribute("rootNode"))

and use the cached serializer instance.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Is there any logic behind that only some constructors use cache? – Leonid Vasilev Nov 12 '15 at 10:50
  • 1
    @LeonidVasilyev probably convenience; it is easy to cache a single-scenario thing; to do the more complex scenarios would involve constructing a hashing algorithm and robust equality check to compare different configurations, for use with a hash-table of some ilk – Marc Gravell Nov 12 '15 at 11:54
  • 3
    That unsuspecting line of code was causing a memory leak in my application too; I have confirmed. Great find! – Dan Feb 10 '16 at 18:20
  • If you are in foreach loop, initialize it (XmlSerializer mySerializer = new ..) before the loop and pass the object (mySerializer ) to the method you are using for deserialization – Shantu Mar 15 '19 at 14:33
  • 1
    As always Marc Gravell to the rescue :) I've been digging through memory dumps for hours. – Mertus Feb 23 '21 at 10:38
  • To help understand what "simplest" constructor is here is a link to documentation: https://learn.microsoft.com/en-us/dotnet/api/system.xml.serialization.xmlserializer?redirectedfrom=MSDN&view=net-6.0#dynamically-generated-assemblies – schwartz Jun 29 '22 at 09:04
10

First off, you should be disposing of your StreamReader even if an exception is thrown (same for XMLObj). Use the using statement. Currently you will not dispose when an exception is thrown.

It is very unlikely that you have a memory leak. More likely, the runtime simply did not elect to collect memory yet. Even GC.Collect will not necessarily cause memory to be released.

I have run into similar situations when processing very large XML files (multi-GB). Even though the runtime grabs most available memory, it does release it when memory pressure warrants.

You can use the memory profiler in Visual Studio to see what memory is allocated, and in what generation it resides.

UPDATE

The comment from @KaiEichinger is worth investigating. It indicates that the XmlSerializer may be creating a new cached object definition for every loop iteration

XMLSerializer constructor creates the temporary assembly for the type to be serialized using reflection and since code generation is expensive the assembly is cached in the memory on per type basis. But many times root name will be changed and can be dynamic and it will not cache the dynamic assembly. So whenever the above line of code is called it loads the new assembly every time and will stay in the memory until AppDomain is unloaded.

Matt
  • 74,352
  • 26
  • 153
  • 180
Eric J.
  • 147,927
  • 63
  • 340
  • 553
  • 1
    This worked! Thanks a lot. It was an issue with the constructor of the XmlSerializer being in the loop. Once taken out, the memory is stable. – Alex999 May 27 '14 at 19:26
9

From MSDN:enter link description here

To increase performance, the XML serialization infrastructure dynamically generates assemblies to serialize and deserialize specified types. The infrastructure finds and reuses those assemblies. This behavior occurs only when using the following constructors:

XmlSerializer.XmlSerializer(Type)

XmlSerializer.XmlSerializer(Type, String)

If you use any of the other constructors, multiple versions of the same assembly are generated and never unloaded, which results in a memory leak and poor performance. The easiest solution is to use one of the previously mentioned two constructors. Otherwise, you must cache the assemblies in a Hashtable, as shown in the following example.

=> So to fix it you have to use this constructor XmlSerializer xml = new XmlSerializer(typeof(XMLObj)) instead of XmlSerializer xml = new XmlSerializer(typeof(XMLObj), new XmlRootAttribute("rootNode"));

and add root XML attribute into XMLObj class.

[Serializable()]
[XmlRoot("root")]
public class XMLObj: IDisposable
{
    [XmlElement("block")]
    public List<XMLnode> nodes{ get; set; }

    public XMLObj() { }

    public void Dispose()
    {
        nodes.ForEach(n => n.Dispose());
        nodes= null;

        GC.SuppressFinalize(this);
    }
}
Alex Nguyen
  • 1,032
  • 12
  • 27
5

I'm using a "cache" class to avoid instantiating xmlserializer each time you need to serialize something (also added a XmlCommentAttribute for adding comments to the serialized properties in the xml output), to me it works like sharm, hope help someone with this:

 public static class XmlSerializerCache
{
    private static object Locker = new object();
    private static Dictionary<string, XmlSerializer> SerializerCacheForUtils = new Dictionary<string, XmlSerializer>();

    public static XmlSerializer GetSerializer<T>()
    {
        return GetSerializer<T>(null);
    }
    public static XmlSerializer GetSerializer<T>(Type[] ExtraTypes)
    {
        return GetSerializer(typeof(T), ExtraTypes);
    }
    public static XmlSerializer GetSerializer(Type MainTypeForSerialization)
    {
        return GetSerializer(MainTypeForSerialization, null);
    }
    public static XmlSerializer GetSerializer(Type MainTypeForSerialization, Type[] ExtraTypes)
    {
        string Signature = MainTypeForSerialization.FullName;
        if (ExtraTypes != null)
        {
            foreach (Type Tp in ExtraTypes)
                Signature += "-" + Tp.FullName;
        }

        XmlSerializer XmlEventSerializer;
        if (SerializerCacheForUtils.ContainsKey(Signature))
            XmlEventSerializer = SerializerCacheForUtils[Signature];
        else
        {
            if (ExtraTypes == null)
                XmlEventSerializer = new XmlSerializer(MainTypeForSerialization);
            else
                XmlEventSerializer = new XmlSerializer(MainTypeForSerialization, ExtraTypes);

            SerializerCacheForUtils.Add(Signature, XmlEventSerializer);
        }
        return XmlEventSerializer;
    }

    public static T Deserialize<T>(XDocument XmlData)
    {
        return Deserialize<T>(XmlData, null);
    }
    public static T Deserialize<T>(XDocument XmlData, Type[] ExtraTypes)
    {
        lock (Locker)
        {
            T Result = default(T);
            try
            {
                XmlReader XmlReader = XmlData.Root.CreateReader();
                XmlSerializer Ser = GetSerializer<T>(ExtraTypes);
                Result = (T)Ser.Deserialize(XmlReader);
                XmlReader.Dispose();
                return Result;
            }
            catch (Exception Ex)
            {
                throw new Exception("Could not deserialize to " + typeof(T).Name, Ex);
            }
        }
    }
    public static T Deserialize<T>(string XmlData)
    {
        return Deserialize<T>(XmlData, null);
    }
    public static T Deserialize<T>(string XmlData, Type[] ExtraTypes)
    {
        lock (Locker)
        {
            T Result = default(T);
            try
            {

                using (MemoryStream Stream = new MemoryStream())
                {
                    using (StreamWriter Writer = new StreamWriter(Stream))
                    {
                        Writer.Write(XmlData);
                        Writer.Flush();
                        Stream.Position = 0;
                        XmlSerializer Ser = GetSerializer<T>(ExtraTypes);
                        Result = (T)Ser.Deserialize(Stream);
                        Writer.Close();
                    }
                }
                return Result;
            }
            catch (Exception Ex)
            {
                throw new Exception("Could not deserialize to " + typeof(T).Name, Ex);
            }
        }
    }

    public static XDocument Serialize<T>(T Object)
    {
        return Serialize<T>(Object, null);
    }
    public static XDocument Serialize<T>(T Object, Type[] ExtraTypes)
    {
        lock (Locker)
        {
            XDocument Xml = null;
            try
            {
                using (MemoryStream stream = new MemoryStream())
                {
                    XmlSerializerNamespaces ns = new XmlSerializerNamespaces();
                    ns.Add("", "");

                    using (StreamReader Reader = new StreamReader(stream))
                    {
                        XmlSerializer Serializer = GetSerializer<T>(ExtraTypes);
                        var settings = new XmlWriterSettings { Indent = true };
                        using (var w = XmlWriter.Create(stream, settings))
                        {
                            Serializer.Serialize(w, Object, ns);
                            w.Flush();
                            stream.Position = 0;
                        }
                        Xml = XDocument.Load(Reader, LoadOptions.None);

                        foreach (XElement Ele in Xml.Root.Descendants())
                        {
                            PropertyInfo PI = typeof(T).GetProperty(Ele.Name.LocalName);
                            if (PI != null && PI.IsDefined(typeof(XmlCommentAttribute), false))
                                Xml.AddFirst(new XComment(PI.Name + ": " + PI.GetCustomAttributes(typeof(XmlCommentAttribute), false).Cast<XmlCommentAttribute>().Single().Value));
                        }

                        Reader.Close();
                    }
                }
                return Xml;
            }
            catch (Exception Ex)
            {
                throw new Exception("Could not serialize from " + typeof(T).Name + " to xml string", Ex);
            }
        }
    }
}

[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public class XmlCommentAttribute : Attribute
{
    public string Value { get; set; }
}
Danilow
  • 390
  • 3
  • 9
2

I have recently faced the same issue with the latest .NET Core 3.1 and caching XMLSerializer (Proposed here) did the trick. The worst thing with this memory leak is that is not clearly localizable from the memory dump, I tried dotMemory from Jetbrains and everything seemed ok there according the result from the analyzed dump, but the amount of memory used by the app (size of dump) and the amount of memory used by the app showed in dotMemory report were significantly different. dotMemory showed only a few MB of memory used by the APP. I originaly thought that the problem was caused by the WCF, which is really tricky to make it work in .NET Core when the contract (WSDL) uses different encoding than utf-8 and especially when the contract contains dots in names of methods (The server-side was written in PHP) That would be not a problem with .Net Framework, but tooling for .Net Core is different. I had to adjust WSDL manually and add some classes missing in .Net Core implementation to get working it for different encoding.

1

I think moving the XMLSerializer constructor outside the loop and caching its result will fix it, explanation here

Peter Wishart
  • 11,600
  • 1
  • 26
  • 45