8

I am going to make a GUI that will have dynamically created sets of controls with events assigned to them. I will need to add and remove those controls at runtime. It will look like this:

FlowLayoutPanel.Controls.Clear();
<< add new controls, assigning Click events with += >>

I have heard that assigning event handlers with += can cause memory leaks (more specificly, memory will not be freed until application has exited). I want to avoid this. I know i can write some functions like here How to remove all event handlers from a control to find all event handlers and remove them but it looks very complex.

Is there another way? Does calling Dispose help remove those event handlers? Can you destroy objects to force their memory to be freed like in C/C++?

Thanks!

PS: Problem is, i dont know what event to detach. I will create lots of labels and add different kinds of onclick events to them. When its time to clean the flow layout panel, there is no way to know what event handler was attached to which label.

This is the example code (_flowLP is a FlowLayoutPanel) - this Refresh() function is ran multiple times before the application exits.

    private void Refresh()
    {
        Label l;
        Random rnd = new Random();

        // What code should i add here to prevent memory leaks
        _flowLP.Controls.Clear();

        l = new Label();
        l.Text = "1";
        if (rnd.Next(3) == 0) l.Click += Method1;
        if (rnd.Next(3) == 0) l.Click += Method2;
        if (rnd.Next(3) == 0) l.Click += Method3;
        _flowLP.Controls.Add(l);

        l = new Label();
        l.Text = "2";
        if (rnd.Next(3) == 0) l.Click += Method1;
        if (rnd.Next(3) == 0) l.Click += Method2;
        if (rnd.Next(3) == 0) l.Click += Method3;
        _flowLP.Controls.Add(l);

        l = new Label();
        l.Text = "3";
        if (rnd.Next(3) == 0) l.Click += Method1;
        if (rnd.Next(3) == 0) l.Click += Method2;
        if (rnd.Next(3) == 0) l.Click += Method3;
        _flowLP.Controls.Add(l);

        l = new Label();
        l.Text = "4";
        if (rnd.Next(3) == 0) l.Click += Method1;
        if (rnd.Next(3) == 0) l.Click += Method2;
        if (rnd.Next(3) == 0) l.Click += Method3;
        _flowLP.Controls.Add(l);

        l = new Label();
        l.Text = "5";
        if (rnd.Next(3) == 0) l.Click += Method1;
        if (rnd.Next(3) == 0) l.Click += Method2;
        if (rnd.Next(3) == 0) l.Click += Method3;
        _flowLP.Controls.Add(l);

        l = new Label();
        l.Text = "6";
        if (rnd.Next(3) == 0) l.Click += Method1;
        if (rnd.Next(3) == 0) l.Click += Method2;
        if (rnd.Next(3) == 0) l.Click += Method3;
        _flowLP.Controls.Add(l);
    }
Community
  • 1
  • 1
Istrebitel
  • 2,963
  • 6
  • 34
  • 49
  • Regarding your edit: If the methods added to an event's invocation list can't be determined for sure, you could keep the delegates that have been added to event handlers in a collection somewhere and use that to remove the delegates when the time comes. Or, if you know you're clearing the events' invocation lists entirely, just clear them entirely. – phoog Apr 09 '12 at 21:02

3 Answers3

3

This is mainly going to be a worry when you attach a shorter lived event consumer to a longer lived event producer. If they have similar lives or are the opposite of what I described, it's a non-issue.

In the case where you do worry about this, just use -= to detach from the event. That removes the reference created by attachment and helps avoid this type of memory issue.

Edit: Since the comments are getting a little long, I'll post some follow up here. When you attach to an event, what you're doing is hanging a reference to yourself on the event provider. So, for instance, if you had a Clock class with a StrikesMidnight event and you subscribe to that event from a class called Bedtime, the actual mechanics of Bedtime saying clock.StrikesMidnight += this.HandleMidnight; is that you're assigning clock a reference to yourself. It's as if Clock had an object property and you said clock.ObjectProperty = this;

So, in a case where the Bedtime class is short-lived and goes out of scope, Bedtime shows up, hangs a reference to itself on Clock, and then goes out of scope. Problem is, Clock still has a reference to it, so even though it's out of scope, Garbage Collector won't collect Bedtime.

....

That's the background. In your case, you're creating a label and attaching a reference to yourself to it (via your "MethodX" handlers). When refresh is called, you clear your list of labels (meaning they go out of scope). They go out of scope and they have references to your class via its MethodX handlers, but so what? Them having references does not prevent them from being GC'ed. Nobody is holding references to them in your code, so the GC will do its work on them and you won't leak memory.

Erik Dietrich
  • 6,080
  • 6
  • 26
  • 37
  • 1
    But i dont know what event to detach! What do i do then? – Istrebitel Apr 09 '12 at 18:09
  • 1
    It's just the inverse of what you're doing with your += statements. Each time you attach a local event handler to some collaborator's event, remember to detach with -= when you no longer need to be aware of the event being raised. You can do this in a dispose() method if you want, though that is not strictly necessary. – Erik Dietrich Apr 09 '12 at 18:10
  • yes but i do not know which method to detach. say, i have 10 methods, one of which was attached to the 5th of 20 label some time ago based on conditions at that point of time. Now i need to remove all labels from the flowlayoutpanel. How do i know which of 10 event handlers do i have to -=? – Istrebitel Apr 09 '12 at 18:37
  • Can you post some sample code to illustrate what you're talking about? That might make it easier for me to respond with specifics. – Erik Dietrich Apr 09 '12 at 18:40
  • All code would take alot of space, and it wont be relevant. Just think of this: 20 labels are created, each gets a random Click event handler out of list of 10 event handlers called Foo1..Foo10, and each gets added to flowlayoutpanel. Then, in 2 minutes, i need to clear the flowlayout panel and do it again. How do i make sure memory owned by previous 20 labels is property GCed? I cannot use -= because they were attached randomly and i didnt store the information about who got what in memory. – Istrebitel Apr 09 '12 at 19:33
  • Posting the code was for your sake, not mine. I understand the issue, but it's easier to describe if there are actual things I can point you at. But, c'est la vie. When you assign the passive voice -- the labels "are created", that's part of your problem. I don't know who creates them, but take charge of that. And, when they get created, cache references to them so that you can keep them around. Then, later, you can unhook from them. – Erik Dietrich Apr 09 '12 at 19:36
  • Sorry, i do understand now that i maybe wasnt clear. I added the code. Please see if this explains the problem. – Istrebitel Apr 09 '12 at 20:15
  • Yes, that helps. You don't need to do anything with your code. You're good. The labels are shorter lived than the event handlers, so they go out of scope and can't keep anything from being GC-ed. So, you're in good shape. – Erik Dietrich Apr 09 '12 at 20:28
0

All of the controls should get cleaned up by the garbage collector as long as you dispose the containing form.

Subscribing to event on the controls will not keep the control alive, because the control has a reference to the handling delegate; the delegate does not have a reference to the control.

The situation where an event subscription will keep a control from being cleaned up, is where some code in the control is subscribing to an event outside of the form instance it is contained in. For example, if a custom combo-box subscribes to an event on a static class to let the control know when its options list should be updated. If the custom-control does not unwire this event, it will be referenced by the static classes event register for the duration of the application. Since the control will have a reference to its container (and so on) the entire form will probably stay in memory. In cases like this, the control should unwire the event in its Dispose method. Subscribing to events on static classes or long lived instance classes should always raise a red flag and be explicitly unwired when no longer needed.

In forms, as long as all of your events are wired to instance methods on the form class or on objects whose scope is controlled by the form instance, then the controls will get cleaned up when the object graph that the form is rooting goes out of scope. Just be careful that external classes do not hold a reference to the form after it is no longer needed.

Preston McCormick
  • 752
  • 5
  • 17
  • In other words, if an object that goes out of scope has another object's method subsctibed to its event, it will be disposed and GCed just fine, but if an object that goes out of scope has some of its methods subscribed to some other live object's event, then it wont be GCed? – Istrebitel Apr 09 '12 at 19:31
  • That is pretty much correct. They will still get garbage collected if the object that they are subscribing to events on becomes eligible for collection. Circular references between objects otherwise eligible to be collected will not keep them from getting collected. – Preston McCormick Apr 09 '12 at 21:11
0

My first suggestion would be to not pre-optimize. Make sure this is a problem before you try to solve it.

In regards to Dispose(): Dispose should ONLY be used to free unmanaged resources before an object is garbage collected. See http://msdn.microsoft.com/en-us/library/system.idisposable.aspx for more information.

To prevent event handlers from holding onto references to your controls you will need to have your controls unsubscribe from all event handlers that they subscribe to when they are created. HINT: If you're adding event handlers through the GUI investigate the auto-generated code to see what you need to unsubscribe from before calling FlowLayoutPanel.Controls.Clear(). A clean(-er?) way of doing this would be to create your own control which inherits from the desired control and add a `Cleanup()' method that unsubscribes from any event handler that has been subscribed to (you should know which event handlers have been subscribed to - either because you wrote the code or it's been generated for you).

Dan Busha
  • 3,723
  • 28
  • 36