27

We are developing a big .NET Windows Forms application. We are facing a memory leak/usage problem in that despite we are disposing the forms.

The scenario is like:

  1. Our application is using 60 KB of memory with a list of records displaying in a grid.
  2. When the user clicks on a record it opens a form, myform.showDialog, show the details. The memory jumps from 60 KB to 105 MB.
  3. Now we close the form myform to get back to grid, and dispose that form and set it to null. Memory remains at 105 MB.
  4. Now if we again perform step 2, it would jump from 105 MB to 150 MB and so on.

How can we free up the memory when we close myForm?

We have already tried GC.Collect(), etc., but without any result.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Kashif
  • 14,071
  • 18
  • 66
  • 98
  • 1
    How far does the "and so on" go? Honestly, you aren't talking about significant amounts of memory here, and the GC likely won't even try to free memory at those levels. – Andrew Barber Oct 08 '10 at 14:42
  • 1
    Are you sure it is a leak? Does it happen when compiled in Release mode? Think that the .Net will not free memory in the same moment than you free your form like in c++, it will keep the memory for future use. Try to use the application in a longer session, and try to minimize the window (this sometimes forces a release somehow). – jmservera Oct 08 '10 at 14:43
  • @Andrew Barber. Sometimes it consume more than 1 GB. – Kashif Oct 08 '10 at 14:50
  • @jmservera, Minimizing the window and using longer session also not work. – Kashif Oct 08 '10 at 14:53
  • 2
    I think you mean "60M to 105M" instead of "60K to 105K". 150K is nothing. – We Are All Monica Oct 08 '10 at 14:59
  • @jmservera minimizing the window can *appear* effective when you're using Task Manager to monitor usage--but in reality it does very little. – STW Oct 08 '10 at 15:12
  • @jnylen, you are right. I have edited it. – Kashif Oct 08 '10 at 15:12
  • @STW you are right, it was just to check if he was "feeling a leak" or was a real one. – jmservera Oct 08 '10 at 16:30
  • 1
    This is not a leak, it is just memory usage. – leppie Oct 09 '10 at 06:56
  • @leppie: reread the question, what you said doesn't make any sense. – We Are All Monica Oct 12 '10 at 02:20
  • @jnylen: Read up on conservative garbage collectors, then it will make sense. – leppie Oct 12 '10 at 03:56
  • 4
    I have the same issue and the memory usage can grow up until 1.5 Gb (Gigabyte) – Miguel Aug 20 '13 at 14:10
  • does the myForm contains any datagridview ? are you re-assigning datasource of the parent form's datagridview ? DataGridView is a major cause of memory leak in windows forms applications. If the above question's answer is yes then read this question to handle datagridview memory leak http://stackoverflow.com/questions/1702087/apparent-memory-leak-in-datagridview – Haider Ali Wajihi Jun 28 '16 at 08:29

8 Answers8

30

The first place to look for leaks is in event-handling rather than missing Dispose() calls. Say your container (the parent form) loads a child form and adds a handler for an event of that child form (ChildForm.CloseMe).

If the child form is intended to be cleared from memory then this event handler must be removed before it is a candidate for garbage collection.

STW
  • 44,917
  • 17
  • 105
  • 161
  • 5
    +1 -- Beat me to it. This is definitely the most common form of memory leak I've run into with .NET – Mark Simpson Oct 08 '10 at 14:39
  • 4
    Yep. We had a *ton* of WinForms code when we started getting reports of `OutOfMemoryExceptions` which led to months of end of hunting down these types of leaks. Definately a pain! I'm still upset that MS's documentation on `AddHandler / +=` calls doesn't have a huge, blinking warning about needing to guarantee `RemoveHandler / -=` is called – STW Oct 08 '10 at 14:43
  • +1 for the most common cause of a memory leak in .NET. I wish MS had made the `void Dispose(bool disposing)` pattern ( http://msdn.microsoft.com/en-us/library/fs2xkftw.aspx ) much more obvious in the documentation as well as the `+=` and `-=` for event handlers. – Travis Gockel Oct 08 '10 at 14:55
  • @Travis : funny you should mention `Dispose(bool)`. One of my first SO questions was about that: http://stackoverflow.com/questions/773165/why-does-vs2005-vb-net-implement-the-idisposable-interface-with-a-disposedisposi – STW Oct 08 '10 at 14:58
  • Just noting, since the GC does handle circular references, unless you have an EventHandler rooted outside of the form control hierarchy, it's not necessary to actually remove your handlers explicitly (-=) – Jeff Oct 08 '10 at 14:59
  • @Jeff -- correct; entire portions of the object graph can be collected provided they are unreachable. However, I'd advise adding the extra `RemoveHandler / -=` calls just to help play it safe--especially if GC is not well understood – STW Oct 08 '10 at 15:06
12

Disposing the forms is not necessarily a guarantee that you are not leaking memory. For example, if you are binding it to a dataset but you are not disposing of the dataset when you are done, you will probably have a leak. You may need to use a profiling tool to identify which disposable resources are not being released.

And btw, calling GC.Collect() is a bad idea. Just saying.

Otávio Décio
  • 73,752
  • 17
  • 161
  • 228
  • 4
    +1 `GC.Collect()` being called is frequently evidence of *bad programming* (even if the bad programming is just the GC.Collect() call itself) – Andrew Barber Oct 08 '10 at 14:36
  • 1
    @Andrew : you can conceive of no case where calling GC.Collect() would be a good idea? – Hogan Oct 08 '10 at 14:41
  • 1
    @Hogan : If I could conceive of **no** case for calling `GC.Collect()`, I would have used the word *always* rather than *frequently*. – Andrew Barber Oct 08 '10 at 14:43
  • 3
    @Hogan -- The guidelines are quite clear: DON'T! Sure there are valid reasons to do it, but those reasons exist for the extreme minority of applications (such a small minority that you can safely assume you aren't in it). So, again *DON'T!*. GC is a very expensive and slow operation (especially pre .NET 4), it is also a fully automated process--so manually calling it is wasteful. – STW Oct 08 '10 at 14:49
  • 2
    @Hogan - It was pointed out in another answer that it is possible to make matters worse by calling GC.Collect() due to generation promotion. – Otávio Décio Oct 08 '10 at 14:52
  • Technically, there is no need to call Dispose() on Datasets and DataTables, as they suppress finalization in their constructors. They don’t have unmanaged resources; so despite the fact that MarshalByValueComponent makes allowances for unmanaged resources, these particular implementations don’t have the need and can therefore forgo finalization. – Patrick Oct 08 '10 at 15:24
  • @Andrew -- fair point. @STW, @Otávio Décio -- missed my point. :D – Hogan Oct 08 '10 at 17:23
12

The most common source of memory leaks in Windows Forms applications is event handlers that remain attached after form disposal...so this is a good place to start your investigation. Tools like http://memprofiler.com/ can help greatly in determining roots for instances that are never being GC'd.

As for your call to GC.Collect

  1. It's definitely not good to do this in practice
  2. In order to make sure your GC collect really does collect as much as possible, you need to make multiple passes and wait for pending finalizers, so the call is truly synchronous.

    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
    GC.WaitForPendingFinalizers();
    

Also, anything that holds on to your instance of your form will keep the form around in memory, even after it's been Closed and Disposed.

Like:

static void Main() {
    var form = new MyForm();
    form.Show();
    form.Close();

    // The GC calls below will do NOTHING, because you still have a reference to the form!
    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
    GC.WaitForPendingFinalizers();

    // Another thing to not: calling ShowDialog will NOT 
    // get Dispose called on your form when you close it.
    var form2 = new MyForm();
    DialogResult r = form2.ShowDialog();

    // You MUST manually call dispose after calling ShowDialog! Otherwise Dispose 
    // will never get called.
    form2.Dispose();

    // As for grids, this will ALSO result in never releasing the form in
    // memory, because the GridControl has a reference to the Form itself
    // (look at the auto-generated designer code).
    var form3 = new MyForm();
    form3.ShowDialog();
    var grid = form3.MyGrid;

    // Note that if you're planning on actually using your datagrid
    // after calling dispose on the form, you're going to have
    // problems, since calling Dipose() on the form will also call
    // dispose on all the child controls.
    form3.Dispose();
    form3 = null;
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Jeff
  • 35,755
  • 15
  • 108
  • 220
  • 2
    It may not reflect the released memory in the task manager until the second GC call or until you wait for pending finalizers. Your statement about Disposing after you call ShowDialog is incorrect. See "Why you must Dispose() after calling ShowDialog()" under the Microsoft article http://msdn.microsoft.com/en-us/library/system.windows.forms.form.showdialog%28VS.90%29.aspx – Jeff Oct 08 '10 at 15:07
  • 1
    you mentioned the Task Manager, which is a **TERRIBLE** means of measuring .NET memory usage. Use `perfmon`'s Process metrics for things like Private Bytes instead. Task manager used to show us ~95% memory reduction just by minimizing and restoring the application, just as evidence of how off-the-mark its reporting is. – STW Oct 08 '10 at 15:11
  • Yes, I'm aware of the "minimize and suddenly your application is mega memory conservative" effect :) – Jeff Oct 08 '10 at 15:16
  • You're correct, ShowDialog() does only Hide when the form is closed; I retract my comment. It is still true that form becomes eligible for collection after the last time it is referenced within the method, however. – Dan Bryant Oct 08 '10 at 16:46
  • It took me four years of WinForms programming to realize the bit about ShowDialog()...and it was only a very painful experience that taught me that. :D – Jeff Oct 08 '10 at 17:35
1

First, check for references to your form. Does your form subscribe to any events? Those count as references, and if the event publisher lives longer than your form, then it will keep your form around (unless you unsubscribe).

It could also be a coincidence -- .NET allocates memory in segments, I believe, so you may not see your working set go down with every form release (the memory is released by your form, but is still held for the next allocation by your application). Since your memory allocation is, at least, one abstraction away, you won't always get behavior where your working set goes up and down by the exact number of bytes you allocate.

A way to test this is to create a lot of instances of your form and release them -- try to magnify the leak so that you allocating and releasing 100's of instances. Does your memory continue to step up without going down (if so, you have a problem), or does it eventually return to close to normal? (probably not a problem).

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
JMarsch
  • 21,484
  • 15
  • 77
  • 125
1

I recently had a similar issue where a running timer kept the form in memory, even though the form was closed. The solution was to stop the timer before closing the form.

Peet Brits
  • 2,911
  • 1
  • 31
  • 47
0

I have not seen your code but this is the most likely scenario:

1) Your form gets closed but there is a reference to it hanging there and cannot be garbage collected.

2) You are loading some resource that does not get freed up

3) You are using XSLT and compile it everytime you transform

4) You have some custom code that gets compiled and loaded at runtime

Aliostad
  • 80,612
  • 21
  • 160
  • 208
  • 1
    +1 Also, the .NET GC works in 'generations'; even if you are 100% sure there are no remaining references to a form or other object, calling `GC.Collect()` does not guarantee the memory will be freed. Objects which survive a GC can be promoted to a higher generation, meaning they will be checked less frequently for collection. – Andrew Barber Oct 08 '10 at 14:41
  • @Andrew - very good point. I've heard of cases where calling GC.Collect() makes the situation worse exactly because of promotion. – Otávio Décio Oct 08 '10 at 14:46
  • It might be what you're getting at, but there are certain constructors for `XmlSerializer` which also dynamically generate and load a new assembly every time they're called--if you don't manually cache and retrieve their results then they can quickly become a leak – STW Oct 08 '10 at 14:46
  • 1st generation is only 2 MB (although nowadays it could be different from machien to machine) – Aliostad Oct 08 '10 at 14:47
0

Some third party controls have errors in their code. It may not be your issue if you're using some of those controls.

Jay
  • 13,803
  • 4
  • 42
  • 69
0

Check that you have completely removed all the references to the form. Sometimes may happen that you have some hidden references done that you did not notice.

For example: if you attach to external events from your dialog, that is, to events of an external window, if you forget to dettach from them you will have a remaining reference to your form that will never disappear.

Try this code into your dialog (example bad code...):

   protected override void OnLoad(EventArgs e)
   {
       Application.OpenForms[0].Activated += new EventHandler(Form2_Activated);
       base.OnLoad(e);
   }

   void Form2_Activated(object sender, EventArgs e)
   {
       Console.WriteLine("Activated!");
   }        

And open and close the dialog many times, you will se the number of strings in the console grow for each call. This means that the form remains in memory even if you called to dispose (that should only be used for releasing unmanaged resources, i.e.: closing a file and things like this).

jmservera
  • 6,454
  • 2
  • 32
  • 45