Code Review: OutOfMemoryException problem

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 开发者_JAVA技巧(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!

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).

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();
           }
        }


}

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

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">
  <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>

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.

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.