1

I have piece of code that the Ants profiler points to that is causing a memory leak. I have monitored the application over a period of 1 week, but the memory seems to be increasing and not coming back. SO I'm a bit concerned with the below code.

      public void printXML(XmlDocument doc)
        {
            //System.Threading.Timer timer = null;
            XmlWriterSettings settings = new XmlWriterSettings { Encoding = Encoding.UTF8, Indent = true };
            new System.Threading.Timer((_) =>
            {
                using (var writer = XmlWriter.Create(_folderDestination, settings))
                {
                    //                Task.Delay(15000).ContinueWith(_ => doc.Save(writer));                
                    doc.Save(writer);
                }
            }).Change(15000, -1);

        }

Everytime the method printXML is called it would write the doc to the _folderDestination after a period of 15secs. This is what I want to achieve. But the above code seems to be leaking memory and the memory never returns back. So if someone could help to optimize it, it would be great.

user726720
  • 1,127
  • 7
  • 25
  • 59
  • 3
    Side note: Your `Timer` is eligible to garbage collection. You need to store a reference of timer to prevent it from being GC'd. – Sriram Sakthivel Nov 06 '14 at 06:31
  • I'm curious, why have you commented the solution with `Task.Delay`? As @SriramSakthivel mentioned, your timer will collected by GC, and your code will never execute. `Task.Delay` is much easier here. – Dennis Nov 06 '14 at 06:47
  • @Dennis: The code executes and I have the 15sec effect, but I'm more concerned for the memory issue – user726720 Nov 06 '14 at 06:52
  • @user726720: this code *may* execute within debugger session, also, it *may* execute without debugger, but the probability of its execution in release configuration is near 0. – Dennis Nov 06 '14 at 06:54
  • @SriramSakthivel: I belive garbage collection is not an issue here, please correct me if I'm wrong (which I am most of the time, lol), because the code does execute. So should the timer be GC'ed after execution. If so a new object is placed in memory the next time `printXML` is called. How do i prevent the formation of new objects ? – user726720 Nov 06 '14 at 06:57
  • @Dennis: I have tried that in release configuration and in production for quite some time now. I'm quite right, it executes. – user726720 Nov 06 '14 at 06:58
  • @user726720: you *can't* predict, when the garbage collection will occur. This code *may* work, but it eventually will fail, because the timer will be collected *before* it elapses. – Dennis Nov 06 '14 at 07:03
  • @user726720 My comment is not about memory issue. Honestly there is no information to answer that question. It will be better if you can post a [mvce](http://stackoverflow.com/help/mcve) to reproduce the problem. My above comment says there is a problem in your code which mayn't be obvious, that works now doesn't mean it will continue to work. Refer [this](http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer) – Sriram Sakthivel Nov 06 '14 at 07:04
  • Time whole purpose of using the timer here is to delay execution, and there is no elegant way to dispose it (which causes the memory leak). It maybe easier to just start a new thread that sleeps for x amount of second. Or Reuse the same timer if no two printXML will be running at the same time. – Vince Nov 06 '14 at 07:29
  • @Vince I have 7 different thread's of FSW's running at the same time which can call `printXML` at different times. – user726720 Nov 06 '14 at 07:31
  • @user726720 How about spawning a new thread? See my edited answer. – Vince Nov 06 '14 at 07:49

1 Answers1

2

System.Threading.Timer implements IDisposable.

Wrap it inside a using statement to make sure it gets Disposed properly.

If the purpose of the timer is to delay execution, an alternative way can to use a new Thread and do Thread.Sleep.

    public void printXML(XmlDocument doc)
    {
        var thread = new System.Threading.Thread(new System.Threading.ParameterizedThreadStart(DelayPrint));
        thread.Start(doc);
    }

    void DelayPrint(object param)
    {
        System.Threading.Thread.Sleep(15000);              
        XmlDocument doc = param as XmlDocument;
        // Do Work
    }
Vince
  • 620
  • 1
  • 5
  • 9