views:

132

answers:

5

I have a form with a panel on which I dynamically load multiple user controls. I handle events for each of these controls.

UserControl userControl1 = LoadControl("control.ascx") as UserControl;
userControl1.Event += new ControlEventHandler(userControl_Event);
this.Panel.Controls.Add(userControl1);

UserControl userControl2 = LoadControl("control.ascx") as UserControl;
userControl2.Event += new ControlEventHandler(userControl_Event);
this.Panel.Controls.Add(userControl2);

...

Now when I get rid of the controls on the Panel, I simply do a

this.Panel.Controls.Clear();

Does the Clear() function take care of getting rid of the events or should I be doing

foreach(Control control in this.Panel.Controls)
{
    UserControl userControl = control as UserControl;
    if(userControl != null)
    {
        userControl -= userControl_Event;
    }
}

before I Clear() the content of the Panel?

Basically, i'm looking for a way to load user controls dynamically and handle their events without creating a leak when I get rid of them.

Thanks!

EDIT: Because my controls are created in the Page_Init event of the page (each time, as they are loaded dynamically), is it correct to say that their lifespan cannot be longer than the page's lifespan? From what I understand, the control doesn't exist after a post back. A new one is created each time. Therefore, I shouldn't have to unregister the event because the object it doesn't even exist on the next page load. Is that correct?

A: 

the Garbage Collector should be able to collect them without you having them to unregister

Tim Mahy
+3  A: 

The page will hold references to the dynamically instantiated controls even after the collection has been cleared, which will prevent the controls from being collected until the page is itself collected.

In this particular situation this will work out fine because the life of the page is very short.

However, if this were instead a windows forms app, then the memory would effectively be leaked until the form was released.

In general it is a good idea to unsubscribe your events when you release the objects that the events go to, as this is the source of the vast majority of .net memory leaks.

Jason Coyne
+1  A: 

This page on MSDN answers your question:

Control..::.ControlCollection..::.Clear Method

Quoting it:

Important

Calling the Clear method does not remove control handles from memory. You must explicitly call the Dispose method to avoid memory leaks.

Leniel Macaferi
A: 

It is correct to say that lifespan of usercontrols that are dynamically loaded cannot be live longer than the page it belongs. But we cannot say this true in the case of removing a web control from the page control collection and assigning it to a, for example, session variable may cause web control having longer lifespan than page. So it is not always correct.

It is important that you should unsubscribe from an event if you subscribe to a long life object's event (such as singleton object or objects stored with application, session and cache in the asp.net) with a method of a short life object (such as page, usercontrol or web control etc).

For example

UserControl uc = LoadControl("control.ascx") as UserControl;
SomeObject so=Session["SomeObject"] as SomeObject;
If(so!=null)
{
    so.SomeEvent += new SomeEventHandler(uc.SomeMethod);
}

Here it should be unsubscribed from the event for not to cause a memory leak.

Eventually you dont have to worry about the registered events in your case. They will be collected by the garbage collector.

orka
A: 

The fact that you call controls.Clear in the panel is evidence enough that you should also unregister the events.

To do that, you could create your own control collection, with the unregistration code in the overridden Clear method, than create your own panel that uses your new collection, returned from an overridden Controls property getter.

Jordão