0

I'm populating a ToolStripMenu with entries from an XML file. Each ToolStripItem added to the menu automatically gets about a dozen event handlers assigned to it. The user has the option to change the XML and refresh the menu, which clears everything using:

menuStrip1.Items.Clear();

This repopulates from the XML and refreshes the Form. It appears that the event handlers for all these dynamic items are never removed from memory. Most of the items don't sit directly on menuStrip1 but in dynamically created submenus which reside on menuStrip1.

Nevertheless, I can sit there refreshing the Form and the memory just balloons out of control, adding 5-10 MB per refresh of about 100 dynamic ToolStripItems. It can quickly reach hundreds of MB which crashes the application.

The ToolStripItems also load other things such as properties and icons, but it seems to be the event handlers which use most of the added memory. I commented out all the handlers and the memory became far more (but not completely) sustainable with each refresh.

Is it necessary to enumerate through all the menus and submenus and individually release each event handler to assure it's not sticking around?

Selman Genç
  • 100,147
  • 13
  • 119
  • 184
Kevin Denham
  • 519
  • 1
  • 6
  • 24
  • 1
    You should probably be calling `Dispose` on each of those to release their resources. – tvanfosson Feb 19 '14 at 02:08
  • 1
    If you dispose/delete both the event handler *and* the object it's subscribing to, you should be OK. Otherwise, yeah, you might have to unsubscribe each handler (or look at a different way of subscribing--not compiler-generated events) – Peter Ritchie Feb 19 '14 at 02:08
  • 1
    You may want to look in to the [Weak Event Pattern](http://msdn.microsoft.com/en-us/library/aa970850%28v=vs.110%29.aspx) – Scott Chamberlain Feb 19 '14 at 02:15
  • 1
    see also http://paulstovell.com/blog/weakevents – Peter Ritchie Feb 19 '14 at 02:18
  • One of you should make an answer so I can mark as answered. Long story short seems to be that C# is not as diligent with memory as I thought it was. Thanks for the lesson. – Kevin Denham Feb 19 '14 at 02:22

3 Answers3

1

If you dispose/delete both the event handler and the object it's subscribing to, you should be OK. Otherwise, yeah, you will have to unsubscribe each handler.

Or look at a different way of subscribing--not compiler-generated events. See http://paulstovell.com/blog/weakevents for an example)

Peter Ritchie
  • 35,463
  • 9
  • 80
  • 98
0

Check the ItemRemoved event of your menu strip. it will return an item which is removed from your menu strip. you can dispose it here or your can remove each handler from here.

UPDATED

Once you get the parent menu then you can remove all child menu's event handler by using recursive function call.

private void menuStrip1_ItemRemoved(object sender, ToolStripItemEventArgs e)
{
    foreach (object child in ((ToolStripMenuItem)e.Item).DropDownItems)
    {
        if (child.GetType().Name == "ToolStripMenuItem")
            RemoveHandler((ToolStripMenuItem)child);                
    }
}
private void RemoveHandler(ToolStripMenuItem item)
{
    if (item.HasDropDownItems)
    {
        foreach (Object dropdown in item.DropDownItems)
        {
            if (dropdown.GetType().Name == "ToolStripMenuItem")
                RemoveHandler((ToolStripMenuItem)dropdown);
        }
    }
    else
    {
        //item.Click -= new EventHandler(MenuItem_Clicked);
        //Remove event handlers
    }
}
Shell
  • 6,818
  • 11
  • 39
  • 70
  • Thanks for the suggestion but this only triggers for the the menus directly on menuStrip1, it's really the items in the menus and submenus that have all the event handlers. Right now the only thing I've found is recursively going through the menu hierarchy and individually disposing each item, a pain because each menu has several different types of ToolStripItems which is a casting headache. – Kevin Denham Feb 19 '14 at 23:45
0

A trick that I often use is to create a private _cleanUp = New List<Action>() class-level variable and use this for cleaning up event handlers.

So when I attach an event handler I also add the detach code to the list, like so:

this.NameTextbox.Click += My_Handler;
_cleanUp.Add(() => this.NameTextbox.Click -= My_Handler);

Then on clean up, for your code, I can just do this:

_cleanUp.ForEach(a => a());
_cleanUp.Clear();
menuStrip1.Items.Clear();

I then don't have to remember exactly what was added. And I can add any clean up code I like to the list.

Enigmativity
  • 113,464
  • 11
  • 89
  • 172