7

I have an application I am making that creates a large number of windows controls (buttons and labels etc). They are all being made dynamically through functions. The problem I'm having is, when I remove the controls and dispose them, they are not removed from memory.

void loadALoadOfStuff()
{
    while(tabControlToClear.Controls.Count > 0)
        tabControlToClear.Controls[0].Dispose();
    //I even put in:
    GC.Collect();
    GC.WaitForPendingFinalizers();
    foreach(String pagename in globalList)
        tabControlToClear.Controls.Add(MakeATab(pagename));
}

TabPage MakeATab(string tabText)
{
    TabPage newT = new MakeATab();
    newT.Text = tabText;
    //Fill page with controls with methods like this
    return newT;
}

Now for some reason, this just ain't giving me my memory back, so when the process runs 5 times, I end up with an out of memory violation. I am new to object and control disposal but looking through the vast net still hasn't given me any indications, so if any of you have an idea, I'd be grateful to hear it.

UPDATE: I've been watching the user objects creation and destruction (taskmanager) and noticed I create a tab page, add a click handler, add a panel, add 2 buttons both with click handlers, tooltips and backimages (I think this is where the problem is). The app says it creates 8 new items, but when I run through my dispose, I only remove 4 from memory. I've been trying to remove the event handlers, but it seems to make no difference.

RESOLVED!!! As I was adding new items to the panel, I was passing them a tooltip (stupid, but I'm learning). For anyone else who has the same problem, (thanks to comments and directions from people below. I discovered in order to make a control truly dispose (as I realise I so incorrectly put it) is:

1: IF YOU HAVE A TOOL TIP, MAKE SURE IT'S ACCESSABLE! DO NOT DO WHAT I DID! E.g:

This is WRONG!

TabPage MakeATab(string tabText)
{
    TabPage newT = new MakeATab();
    ToolTip myTip = new ToolTip();
    newT.Text = tabText;
    //Fill page with controls with methods like this
    myTip.SetToolTip(newT, "Something to say");
    return newT;
}

If you do this, you will lose the pointer to the tooltip, and as the tooltip is not a child of the object it's connected to (rather, the tooltip makes a strong reference to the control), then even if you destroy the control, the tooltip that you can't access keeps the object alive.

2: Before anything, call toolTip.RemoveAll(). This removes all it's ties to controls. Note, if you were using this tip for other controls, they just lost their tool tip.

3: Remove any internal controls from the base control.ControlCollection (if they use non managed memory, I guess. I'm doing it cause it's making my app work so...)

4: remove any custom event handlers.

5: finally, dispose the object. I made a quick recursing function that does it quite well.

    private void RecursiveDispose(Control toDispose)
    {
        while (toDispose.Controls.Count > 0)
            RecursiveDispose(toDispose.Controls[0]);

        if (toDispose.BackgroundImage != null)
            BackgroundImage = null;

        if (toDispose.GetType() == typeof(Button))
            toDispose.Click -= [Your Event];
        else if (toDispose.GetType() == typeof(TabPage))
            toDispose.DoubleClick -= [Your Event];
        else if (toDispose.GetType() == typeof(Label))
            toDispose.MouseMove -= [Your Event];

        toDispose.Dispose();
    }

This is extremely crude and there's probably a much better way of doing it, but unless someone can come up with it, this will have to do. Thanks for your help everyone. You might just have saved my entire project.

  • I went one step further, I even created a recursive dispose method for all the items and still... No memory back. private void RecursiveDispose(Control toDispose) { while (toDispose.Controls.Count > 0) RecursiveDispose(toDispose.Controls[0]); toDispose.Dispose(); } This, again, removes all the items visibly and clears the control arrays, yet I'm still not getting any memory back. –  Jan 12 '10 at 06:07
  • 1
    Something else is causing your out of memory error; in general, you never need to call the garbage collector. What is your actual code? – Noon Silk Jan 12 '10 at 06:07
  • Yeah, I just noticed a massive buildup of user objects. But my code only removes some of them. For example, when I run the creation, it makes 8 objects, but when I dispose the objects, it clearss 4 from memory... –  Jan 12 '10 at 07:36
  • I should mention, it's not a clean, 50% loss, sometimes, I build 11 items, clear 6. It's odd. –  Jan 12 '10 at 07:48
  • As for my actual code, it's a bit big to place on here. –  Jan 12 '10 at 08:50

4 Answers4

6

You need to also clear out the reference.

while(tabControlToClear.Controls.Count > 0)
{ 
    var tabPage = tabControlToClear.Controls[0];
    tabControlToClear.Controls.RemoveAt(0);
    tabPage.Dispose(); 

    // Clear out events.

    foreach (EventHandler subscriber in tabPage.Click.GetInvocationList())
    {
        tabPage.Click -= subscriber;
    }
}
ChaosPandion
  • 77,506
  • 18
  • 119
  • 157
  • 1
    Good point, although OP didn't mention any event subs. Forgetting about event handler assignments is a common cause of 'memory leaks'. – Jim L Jan 12 '10 at 06:30
  • Chaos, I might sound a little dense (I'm new, have mercy), but that code doesn't work. –  Jan 12 '10 at 08:20
  • 5
    Maybe "tabPage.Dispose();" should be called after detaching event handlers? – lmsasu Jan 12 '10 at 09:31
4

In this code block you're calling Dispose but not removing the reference:

while(tabControlToClear.Controls.Count > 0)
    tabControlToClear.Controls[0].Dispose();

You need to remove all references to the control (in the Controls collection plus any registered event handlers plus any other references you might have) for a control to be eligible for garbage collection.

Samuel Neff
  • 73,278
  • 17
  • 138
  • 182
  • Thanks for the info, I have some handlers assigned to various controls, but how would I create a recursive clearing function? I have buttons and labels nested on a panel, placed on a tab page added to a tab control. So severely nested. How can I handle the disposal of the object and it's handlers without knowing the type? –  Jan 12 '10 at 07:23
  • Also, I have tooltips attached to these controls, do they need to be disposed? –  Jan 12 '10 at 07:47
0
void loadALoadOfStuff()
{
    while(tabControlToClear.Controls.Count > 0)
        tabControlToClear.Controls[0].Dispose();
    //I even put in:
    GC.Collect();
    GC.WaitForPendingFinalizers();
    foreach(String pagename in globalList)
        tabControlToClear.Controls.Add(MakeATab(pagename));
}

It seems to me you are re-allocating all the tab instances at the end of your test-method, so there clearly is no benefit of disposing them first. Skip the last two lines and see if that helps.

Johannes Rudolph
  • 35,298
  • 14
  • 114
  • 172
  • The purpose of this function is to take away all the controls, release them from memory and replace them with new ones. By removing the last two lines, I remove the reason I wrote the function, which is, to populate the page after disposing the old, unneeded objects. Not just disposal. –  Jan 12 '10 at 07:30
  • If you had mentioned your memory usage increases, I would have suggested what ChaosPandion said. However, I assumed you complained about a constantly high memory usage. – Johannes Rudolph Jan 12 '10 at 08:07
  • Nah, I'm dynamically creating over 1000 user controls... I need that memory back when they clear... –  Jan 12 '10 at 08:16
0

There was been a lot of great answers to the question so far that mention one possible reason the objects are not being freed, they are all things worth checking.

However when I get this type of problem, I use a memory profiler to help track down the reference(s) to the objects, the last time I look at memory profilers, the ANTS Memory Profiler (from RedGate) was one of the best. (They do a 14 day tail that is more than long enough to investigate a single problem like this.)

This is one of the reasons that the Weak Event pattern is used, however that would be a whole new question.

(The lifetime of UI objects when you dynamically modifier an UI is a minefield, well done for taskmanager to check that your code is working as expected)

Community
  • 1
  • 1
Ian Ringrose
  • 51,220
  • 55
  • 213
  • 317
  • As I'm discovering, user control management is much more complex than I first though. (Damned .net lulled me into a false sense of understanding.) One of the issues I think it might be is that I stupidly thought assigning a tooltip to a control somehow put the tooltip object 'in' the control so it would be removed when the control was disposed but no, MS had to go and make it a weird class that encapsulates the object, so even though I've removed the button and tried to dispose it, the tooltip (which I no longer have a pointer to) is still holding the object... This is quite a feat... –  Jan 12 '10 at 09:45
  • @Psytechnic, WinForms have lot of problem like this as it is a managed wrapper round the unmanaged User32 API, however it is still better than trying to use User32 (or MFC) directly from C/C++. WPF is a lot better as it was designed from day one to be used from managed code. – Ian Ringrose Jan 12 '10 at 10:00
  • Would you suggest rewriting the entire application as a WPF app? –  Jan 12 '10 at 11:09
  • @Psytechnic, rewrites cost a lot of money, however if you are only starting out writing the app, then moveing to WPF would be a good option apart from the cost of learning WPF. You can also mix WPF and Winforms in the same app. Expect to spend a few solid weeks to learn WPF well! – Ian Ringrose Jan 12 '10 at 12:59
  • I seem to have the problem on this project sorted, which does save me a costly rewrite, but now I know about WPF and managed controls, I will definitely focus on that in the next project. So now I'm off to play with as much WPF as possible. Thanks for all you help. –  Jan 12 '10 at 13:25