views:

358

answers:

6

I keep getting a 'System.OutOfMemoryException' thrown at the code below. I can't figure out where the memory leak is and it would be a great help if someone could explain what I am doing wrong. Thanks!

lock ((_tabs))
{
    System.IO.StreamReader sr = null;
    System.IO.MemoryStream ms = null;
    try
    {
        Type[] t = { typeof(tsgPublicDecs.tsgClsTab) };
        System.Xml.Serialization.XmlSerializer srl = new System.Xml.Serialization.XmlSerializer(typeof(ArrayList), t);
        ms = new System.IO.MemoryStream();
        srl.Serialize(ms, _tabs);
        ms.Seek(0, 0);
        sr = new System.IO.StreamReader(ms);
        return sr.ReadToEnd();
    }
    finally
    {
        if (((sr != null)))
        {
            sr.Close();
            sr.Dispose();
        }
        if (((ms != null)))
        {
            ms.Close();
            ms.Dispose();
        }
    }
}

EDIT: To Answer a few of the questions:

  • _tabs is not being populated with anything (Which brings up many other questions why its even being used but I'll need to ask the developer who wrote it for that)
  • The line throwing the error is 'srl.Serialize(ms, _tabs);'
  • This error is random and I've been unable to duplicate it myself but letting it run over a few days this will be thrown. Because of this I am unable (don't know how) to get any information beyond the error being thrown.

EDIT 2: Thanks for all the input. Adding usings and looking for other possible memory leaks seems like the best approach. Its great to see how quickly people can lend a hand!

A: 

Depending on how big _tabs is, the fact that you are reading the entire serialised stream via sr.ReadToEnd() might be causing it.

nichromium
A: 

It's screaming for a using block or two.

lock ((_tabs))
{
        Type[] t = { typeof(tsgPublicDecs.tsgClsTab) };
        System.Xml.Serialization.XmlSerializer srl = new System.Xml.Serialization.XmlSerializer(typeof(ArrayList), t);

        using ( System.IO.MemoryStream ms = new System.IO.MemoryStream())
        {  
           srl.Serialize(ms, _tabs);
           ms.Seek(0, 0);

           using (System.IO.StreamReader sr = new System.IO.StreamReader(ms))
           {
            return sr.ReadToEnd();
           }
        }


}
chris
This code won't work (or compile). A) you don't need the finally block with 'using', and B) you're using the MemoryStream outside of its scope.
ladenedge
This requires a nested using block.
kervin
Further, even if this code was properly formed as kervin mentions, it still wouldn't solve your out of memory issue because it offers purely syntactic adjustments. (Which is not to say that using blocks aren't wonderful things, of course.)
ladenedge
ok, I missed the ms parameter. fixed. There is no finally block in there.
chris
while it is true that this code is cleaner, it is semantically identical in terms of memory usage. The question clearly shows that he is disposing the MemoryStream and StreamReader instances. Why is this the accepted answer?
Marek
A: 
  1. Like others have said, I would suspect _tabs.

  2. Set Visual Studio to break on Exceptions thrown and check its size.

  3. If you're going to simply Dispose, why not use nested 'using' blocks instead of a finally block?

  4. How many threads are running when you get the out of memory exception? Your code block may be hanging at a time when the memory has already been used.

kervin
A: 

try this.. This runs in SnippetCompiler. If it doesnt work for you then there is a cyclic reference in your tsgClsTab class. And, yeah, lookup 'using' ;-)

using System;
using System.Collections;
using System.IO;
using System.Xml.Serialization;

public class MyClass
{
    public static void RunSnippet()
    {
        var _tabs = new ArrayList();

        _tabs.Add(new tsgClsTab(1));
        _tabs.Add(new tsgClsTab(2));
        _tabs.Add(new tsgClsTab(3));
        _tabs.Add(new tsgClsTab(4));

        lock ((_tabs))
        {
            StreamReader sr = null;
            MemoryStream ms = null;
            try
            {
                var srl = new XmlSerializer(typeof (ArrayList),
                                            new Type[] {typeof (tsgClsTab)});
                ms = new MemoryStream();
                srl.Serialize(ms, _tabs);
                ms.Seek(0, 0);
                sr = new StreamReader(ms);
                Console.WriteLine(sr.ReadToEnd());
            }
            finally
            {
                if (((sr != null)))
                {
                    sr.Close();
                    sr.Dispose();
                }
                if (((ms != null)))
                {
                    ms.Close();
                    ms.Dispose();
                }
            }
        }
    }

    #region Helper methods

    public static void Main()
    {
        try
        {
            RunSnippet();
        }
        catch (Exception e)
        {
            string error = string.Format("---\nThe following error occurred while executing the snippet:\n{0}\n---",
                                         e.ToString());
            Console.WriteLine(error);
        }
        finally
        {
            Console.Write("Press any key to continue...");
            Console.ReadKey();
        }
    }

    #endregion

    #region Nested type: tsgClsTab

    public class tsgClsTab
    {
        public tsgClsTab()
        {
        }

        public tsgClsTab(int id)
        {
            Id = id;
        }

        public int Id { get; set; }
    }

    #endregion
}

OUTPUT

<?xml version="1.0"?>
<ArrayOfAnyType xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"&gt;
  <anyType xsi:type="tsgClsTab">
    <Id>1</Id>
  </anyType>
  <anyType xsi:type="tsgClsTab">
    <Id>2</Id>
  </anyType>
  <anyType xsi:type="tsgClsTab">
    <Id>3</Id>
  </anyType>
  <anyType xsi:type="tsgClsTab">
    <Id>4</Id>
  </anyType>
</ArrayOfAnyType>
Sky Sanders
+2  A: 

Are you sure that the problem is here?

Aren't you leaking memory somewhere else?

OutOfMemoryException can be thrown on memory allocation from anywhere, and it may be just a coincidence that it happened here.

Try to profile the application in a memory profiler to see if you are holding some references that will cause your memory to "leak".

Try to remove as much RAM from your testing machine as possible (depending on OS, try to go down to 256/128 MB on XP) and run repetitive use cases many times (not necessarily only the use case one that runs this code).

Marek
A: 

Here's another suggestion, if you can swing it. Replace _tabs' declaration (which I assume is declared as an ArrayList somwhere) with a strongly-typed generic as such:

old: private ArrayList _tabs = new ArrayList(); new: private List<tsgPublicDecs.tsgClsTab> _tabs = new List<tsgPublicDecs.tsgClsTab>();

And adjust the XmlSerializer instantiation similarly.

That being said, after some pretty basic testing, I couldn't get any OOM exception to occur. Here's what other people have said as simplifying with usings:

    lock (_tabs)
    {
        Type[] t = { typeof(tsgPublicDecs.tsgClsTab) };
        System.Xml.Serialization.XmlSerializer srl = new System.Xml.Serialization.XmlSerializer(typeof(ArrayList), t);
        using (System.IO.Stream ms = new System.IO.MemoryStream())
        {
            srl.Serialize(ms, _tabs);
            ms.Seek(0, 0);
            using (System.IO.TextReader sr = new System.IO.StreamReader(ms))
            {
                return sr.ReadToEnd();
            }
        }
    }

EDIT: well guess what? I was able to make an OutOfMemoryException occur. But only with an arraylist of 10,000 of these items, each one having a reference to the previous one. So it's a pretty complicated object graph when it gets down to it and that might be what you're encountering.

Jesse C. Slicer