Garbage Collection When Using Anonymous Delegates for Event Handling

Garbage collection when using anonymous delegates for event handling

I know that this question is ancient, but hell - I found it, and I figure that others might as well. I'm trying to resolve a related issue, and might have some insight.

You mentioned Dustin Campbell's WeakEventHandler - it indeed cannot work with anonymous methods by design. I was trying to fiddle something together that would, when I realized that a) in 99% of cases I'd need something like this his original solution would be safer, and b) in those few cases where I have to (note: have to, not "want to because lambdas are so much prettier and concise") it's possible to make it work if you get a little clever.

Your example seems like exactly the kind of one-off case where getting a little tricky can result in a fairly concise solution.

public static class Linker {
    public static void Link(Publisher publisher, Control subscriber) {
        // anonymous method references the subscriber only through weak 
        // references,so its existance doesn't interfere with garbage collection
        var subscriber_weak_ref = new WeakReference(subscriber);

        // this instance variable will stay in memory as long as the  anonymous
        // method holds a reference to it we declare and initialize  it to 
        // reserve the memory (also,  compiler complains about uninitialized
        // variable otherwise)
        EventHandler<ValueEventArgs<bool>> handler = null;

        // when the handler is created it will grab references to the  local 
        // variables used within, keeping them in memory after the function 
        // scope ends
        handler = delegate(object sender, ValueEventArgs<bool> e) {
            var subscriber_strong_ref = subscriber_weak_ref.Target as Control;

            if (subscriber_strong_ref != null) 
                subscriber_strong_ref.Enabled = e.Value;
            else {
                // unsubscribing the delegate from within itself is risky, but
                // because only one instance exists and nobody else has a
                // reference to it we can do this
                ((Publisher)sender).EnabledChanged -= handler;

                // by assigning the original instance variable pointer to null
                // we make sure that nothing else references the anonymous
                // method and it can be collected. After this, the weak
                //  reference and the handler pointer itselfwill be eligible for
                // collection as well.
                handler = null; 
            }
        };

        publisher.EnabledChanged += handler;
    }
}

The WPF Weak Event pattern is rumored to come with a lot of overhead, so in this particular situation I wouldn't use it. Furthermore, referencing the core WPF library in a WinForm app seems a little heavy as well.

Will an anonymous-delegate event listener prevent garbage collection?

The following test suggests that, no, the scenario does not result in a memory leak.

public partial class LeakTest : UserControl
{
    public ICommand PopupCommand { get; private set; }

    public LeakTest()
    {
        InitializeComponent();

        PopupCommand = new DelegateCommand(arg =>
        {
            var child = new ChildWindow();
            child.Closed += (sender, args) =>
            {
                System.Diagnostics.Debug.WriteLine("Closed window");
            };

            // when the window has loaded, close it and re-trigger the command
            child.Loaded += (sender, args) =>
            {
                child.Close();
                PopupCommand.Execute(null);
            };
            child.Show();
        });
    }
}

The reason is suggested in the answer to the (Winforms) post linked to by Jwosty:

In your example, the publisher only exists within the scope of your private method, so both the dialog and the handler will be garbage collected at some point after the method returns.

In other words, the memory-leak concern is really the other way around -- the event publisher (the ChildWindow control) holds a reference to the subscriber (the DelegateCommand), but not the other way around. So, once the ChildWindow is closed, the garbage collector will free its memory.

detaching anonymous listeners from events in C# and garbage collection

The publisher of an event retains a strong reference to each subscriber. If the publisher is longer lived than the subscribers, then the subscribers will be pinned in memory while the publisher is present.

In your example, the publisher only exists within the scope of your private method, so both the dialog and the handler will be garbage collected at some point after the method returns.

I would recommend complying with the dot net framework guidelines for publishing an event, which suggests using a protected virtual method to invoke the events.

EventHandlers and Anonymous Delegates / Lambda Expressions

If object X has an event handler whose target is object Y, then object X being alive means that object Y can't be garbage collected. It doesn't stop object X from being garbage collected.

Normally when something is disposed, it will become garbage pretty soon anyway, which means you don't have a problem.

The problem with events and GC is if you forget to remove a subscribed handler from a different object - i.e. you have a listener which is disposed, but will never be garbage collected because there's still a reference to it from the event in a different object.

Anonymous method for event handler not a leak?

Whenever you add a delegate to an event handler, you should remove it later, right?

Not necessarily, no. Often you want the event handler to stay valid for as long as the event itself can be raised - that's certainly very common with UIs.

Is this really an okay practice?

Absolutely, so long as you don't need to unhook the handler. Think about the point at which you'd unhook the event handler. If it's "when the form (or button, or whatever) is elegible for garbage collection" then what benefit is there in removing the handler? Just let it be garbage collected with the form...

Is it necessary to explicitly remove event handlers in C#

In your case, everything is fine. It's the object which publishes the events which keeps the targets of the event handlers live. So if I have:

publisher.SomeEvent += target.DoSomething;

then publisher has a reference to target but not the other way round.

In your case, the publisher is going to be eligible for garbage collection (assuming there are no other references to it) so the fact that it's got a reference to the event handler targets is irrelevant.

The tricky case is when the publisher is long-lived but the subscribers don't want to be - in that case you need to unsubscribe the handlers. For example, suppose you have some data transfer service which lets you subscribe to asynchronous notifications about bandwidth changes, and the transfer service object is long-lived. If we do this:

BandwidthUI ui = new BandwidthUI();
transferService.BandwidthChanged += ui.HandleBandwidthChange;
// Suppose this blocks until the transfer is complete
transferService.Transfer(source, destination);
// We now have to unsusbcribe from the event
transferService.BandwidthChanged -= ui.HandleBandwidthChange;

(You'd actually want to use a finally block to make sure you don't leak the event handler.) If we didn't unsubscribe, then the BandwidthUI would live at least as long as the transfer service.

Personally I rarely come across this - usually if I subscribe to an event, the target of that event lives at least as long as the publisher - a form will last as long as the button which is on it, for example. It's worth knowing about this potential issue, but I think some people worry about it when they needn't, because they don't know which way round the references go.

EDIT: This is to answer Jonathan Dickinson's comment. Firstly, look at the docs for Delegate.Equals(object) which clearly give the equality behaviour.

Secondly, here's a short but complete program to show unsubscription working:

using System;

public class Publisher
{
    public event EventHandler Foo;

    public void RaiseFoo()
    {
        Console.WriteLine("Raising Foo");
        EventHandler handler = Foo;
        if (handler != null)
        {
            handler(this, EventArgs.Empty);
        }
        else
        {
            Console.WriteLine("No handlers");
        }
    }
}

public class Subscriber
{
    public void FooHandler(object sender, EventArgs e)
    {
        Console.WriteLine("Subscriber.FooHandler()");
    }
}

public class Test
{
    static void Main()
    {
         Publisher publisher = new Publisher();
         Subscriber subscriber = new Subscriber();
         publisher.Foo += subscriber.FooHandler;
         publisher.RaiseFoo();
         publisher.Foo -= subscriber.FooHandler;
         publisher.RaiseFoo();
    }
}

Results:

Raising Foo
Subscriber.FooHandler()
Raising Foo
No handlers

(Tested on Mono and .NET 3.5SP1.)

Further edit:

This is to prove that an event publisher can be collected while there are still references to a subscriber.

using System;

public class Publisher
{
    ~Publisher()
    {
        Console.WriteLine("~Publisher");
        Console.WriteLine("Foo==null ? {0}", Foo == null);
    }

    public event EventHandler Foo;
}

public class Subscriber
{
    ~Subscriber()
    {
        Console.WriteLine("~Subscriber");
    }

    public void FooHandler(object sender, EventArgs e) {}
}

public class Test
{
    static void Main()
    {
         Publisher publisher = new Publisher();
         Subscriber subscriber = new Subscriber();
         publisher.Foo += subscriber.FooHandler;

         Console.WriteLine("No more refs to publisher, "
             + "but subscriber is alive");
         GC.Collect();
         GC.WaitForPendingFinalizers();         

         Console.WriteLine("End of Main method. Subscriber is about to "
             + "become eligible for collection");
         GC.KeepAlive(subscriber);
    }
}

Results (in .NET 3.5SP1; Mono appears to behave slightly oddly here. Will look into that some time):

No more refs to publisher, but subscriber is alive
~Publisher
Foo==null ? False
End of Main method. Subscriber is about to become eligible for collection
~Subscriber

C#: Saving a delegate from garbage collection (static no option)

To keep an object from being collected, you must maintain a reference to it.

Static members in static classes live for application lifetime.

For instance class members, depending on your class design and project design, you may take different approaches. Maintaining a static list may not be a bad idea.

My only question is how are you able to figure that the delegate is being collected. Because, if you are able to access it, then you must have maintained a reference of it. In which case, it must have stayed in memory.

Do lambdas assigned to an event prevent garbage collection of the owning object?

No, o will get freed, and so will the lambda function. There are no references to o from anywhere else, so there is no reason that it should not get freed.

---

Related Topics