Will this event-handler code cause a memory leak?

Question

Say I derive a WPF control eg TextBox and I override one of the On- methods eg OnInitialized

Suppose I did this: this.Initialized += delegate { };

If the window containing this control closed - would this cause a memory leak if nothing else is done?

If this does cause a memory leak, would implementing a Finalizer be a sufficient, minimum remedy?

Answer 1

The memory leak I believe you're thinking of is when you attach an event handler to an event on an object (call it "A") and then lose all references to the object ("B") that owns the handler. The garbage collector can still reach object "B" through object "A's" event and so will never collect it. See this question for some more discussion.

In your case, you're attaching a handler from inside the Form. When the Form is closed and you drop all your references to it, there is no way to get to the form from the rest of your code so the GC happily collects it (when it gets around to it). No leak will occur.

Based on your last comment, implementing a finalizer may not do what you think it does. It's just a hook the runtime gives you to perform some cleanup code, and you're better off implementing the IDisposable interface and pattern. One thing I tend to do in my classes that expose events is set the event to null in the Dispose method. I.e.:

class Foo : IDisposable
{
    public event EventHandler SomethingHappened;

    // ... normal IDisposable implementation details left out

    protected virtual void Dispose(bool Disposing)
    {
        if (Disposing)
        {
            SomethingHappened = null;
        }
    }
}

I don't do this in all of my classes, but it's a relatively clean way to remove all handlers from an object when it's going away. You can only do this from inside the class itself.

In any case, if you're going to do work in a Form based on the PreviewMouseLeftButtonDown event, you should instead override the OnPreviewMouseLeftButtonDown method. Attaching event handlers to listen to a class' own events is generally bad form. Do the following instead:

protected override void OnPreviewMouseLeftButtonDown(MouseButtonEventArgs e)
{
    // Actually raise the event to let other classes know it happened
    base.OnPreviewMouseLeftButtonDown(e);

    // your code...
}