125

I have a class that offers up a few events. That class is declared globally but not instanced upon that global declaration--it's instanced on an as-needed basis in the methods that need it.

Each time that class is needed in a method, it is instanced and event handlers are registered. Is it necessary to remove the event handlers explicitly before the method goes out of scope?

When the method goes out of scope, so goes the instance of the class. Does leaving event handlers registered with that instance that is going out of scope have a memory footprint implication? (I'm wondering if the event handler keeps the GC from seeing the class instance as no longer being referenced.)

Kiquenet
  • 14,494
  • 35
  • 148
  • 243
rp.
  • 17,483
  • 12
  • 63
  • 79

2 Answers2

184

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
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 2
    I agree with this but if possible can you briefly elaborate on or preferably refer to an example of what you mean by "but the subscribers don't want to be"? – Peter McG Feb 03 '09 at 07:12
  • -= Does not work. -= Will result in a new delegate, and delegates don't check equality by using the target method, they do an object.ReferenceEquals() on the delegate. The new delegate does not exist in the list: it has no effect (and doesn't throw an error oddly enough). – Jonathan C Dickinson Feb 03 '09 at 08:35
  • 2
    @Jonathan: No, delegates check equality using the target method. Will prove in an edit. – Jon Skeet Feb 03 '09 at 09:30
  • I concede. I got confused with anonymous delegates. – Jonathan C Dickinson Feb 04 '09 at 08:08
  • @JonSkeet: +1 and thank you so much for very nice explanation. You have said "(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." Does that mean such event un-registering should happen out of if(disposing) clause in the Dispose method? I've asked a similar question here (http://stackoverflow.com/questions/7642812/idisposable-implementation-what-should-go-in-if-disposing) – CharithJ Oct 04 '11 at 05:05
  • @CharithJ: If you want the event handler to live for as long as the subscribing object is undisposed, yes. – Jon Skeet Oct 04 '11 at 05:13
  • @JonSkeet: Yes, in the code that I've got they register events in the subscriber's constructor and that should last until subscriber dies. They have developed View-Presenter like implementation in this way and developers sometimes forget to call the Dispose of the Subscriber. So we have got many memory leaks and subscribers live as long as the publishers live. They have implemented Finalize method but the event un-registering is in side the if (disposing) clause. I wasn't sure whether that's okay to move event unregistering out of the if(disposing). Thank you so much for this answer! – CharithJ Oct 04 '11 at 05:21
  • Am i right that annonymous eventhandlers need to be removed as well when they include a reference to an object? – CSharpie Nov 05 '15 at 14:14
  • @CSharpie: That entirely depends on the situation. If the event publisher stays alive, then any event handler with a reference to an object will indeed keep that object alive, yes... – Jon Skeet Nov 05 '15 at 14:16
  • @JonSkeet `Console.WriteLine("Foo==null ? {0}", Foo == null);`? it does not compile. – MaYaN Jun 12 '16 at 14:22
  • I tried the second edit. It only works as stated in release configuration. In debug configuration I get ```No more refs to publisher, but subscriber is alive End of Main method. Subscriber is about to become eligible for collection ~Subscriber ~Publisher Foo==null ? false``` Any idea why? – Sorin Bolos Apr 19 '19 at 11:55
  • @SorinBolos: All kinds of things can differ in debug configuration - and again when actually debugging. – Jon Skeet Apr 19 '19 at 12:24
8

In your case, you are fine. I originally read your question backwards, that a subscriber was going out of scope, not the publisher. If the event publisher goes out of scope, then the references to the subscriber (not the subscriber itself, of course!) go with it and there is no need to explicitly remove them.

My original answer is below, about what happens if you create an event subscriber and let it go out of scope without unsubscribing. It does not apply to your question but I'll leave it in place for history.

If the class is still registered via event handlers, then it is still reachable. It is still a live object. A GC following an event graph will find it connected. Yes, you will want to explicitly remove the event handlers.

Just because the object is out of scope of its original allocation does not mean it is a candidate for GC. As long as a live reference remains, it is live.

Eddie
  • 53,828
  • 22
  • 125
  • 145
  • 1
    I don't believe any unsubscription is necessary here - the GC sees references *from* the event publisher, not to it, and it's the publisher we're concerned about here. – Jon Skeet Feb 03 '09 at 07:00
  • @Jon Skeet: You're right. I read the question backwards. I've corrected my answer to reflect reality. – Eddie Feb 03 '09 at 14:25