Is It Bad to Not Unregister Event Handlers

Is it bad to not unregister event handlers?

If you have A publishing an event, and B subscribing to an event (the handler), then it is only a problem not to unsubscribe if A is going to live a lot longer than B. Basically, the event subscription means that A can still see B, so would prevent it from being garbage collected, and would still fire events on it even if you've forgotten about it (and perhaps Disposed() it).

For example, this is a problem if A is a static event, and your app runs for a while after B dies... ButB will live as long as A , thus B will not be garbage collected.

It is important to note, one might ask the following:

if B lives a lot longer than A, will B keep A from being garbage collected?

And the answer to that is "no". B has no reference to A through the event; A will be collected as normal

Is it always safe to unsubscribe from an event inside the handler?

To clarify the other answer a bit:

Events are based on delegates (in almost all cases). Delegates are immutable. This applies to multicast delegates, too.

When invoking an event the delegate is loaded and then invoked. If the field that the delegate is stored in is modified then this does not affect the already loaded delegate.

It's therefore safe to modify the event from the handler. Those changes will not affect the currently running invocation. This is guaranteed.

All of this only applies to events backed by a delegate. C# and the CLR support custom events that could do anything at all.

React why should I remove event listeners?

The event listeners need to be removed due to following reason.

• Avoid memory leaks, if the browser is not handled it properly.Modern browsers will garbage collect event handlers of removed DOM elements but it is not true in cases of legacy browses like IE which will create memory leaks.
• Avoid collisions of events of components.
• Remove the side effects when the reference are stored in some persistence such as local storage

Here is a good article to get an insights on event listners

Should I always disconnect event handlers in the Dispose method?

Unless you expect the publisher of the event to outlive the subscriber, there's no reason to remove the event handler, no.

This is one of those topics where folk lore has grown up. You really just need to think about it in normal terms: the publisher (e.g. the button) has a reference to the subscriber. If both the publisher and the subscriber will be eligible for garbage collection at the same time anyway (as is common) or if the publisher will be eligible for garbage collection earlier, then there's no GC issue.

Static events cause a GC problem because they're effectively an infinitely-long-lived publisher - I would discourage static events entirely, where possible. (I very rarely find them useful.)

The other possible issue is if you explicitly want to stop listening for events because your object will misbehave if the event is raised (e.g. it will try to write to a closed stream). In that case, yes, you should remove the handler. That's most likely to be in the case where your class implements IDisposable already. It would be unusual - though not impossible - for it to be worth implementing IDisposable just to remove event handlers.

Do I have to remove the handler

To clear up your general question of when to do Dispose and Finalize:

If you have a field in your class that is a IntPtr (or some other kind of unmanaged resource, but a IntPtr is the most common) and it is your classes responsibility to clean up that resource then you need to implement a finalizer. In that finalizer you should deallocate whatever resource the IntPtr points too. If you don't have a IntPtr then the class you are holding on to should be handling its own finalization and would implement IDisposeable (see the next part)

If you have a field in your class that implements IDisposable and your class is responsible for cleaning up after that object your class should also implement IDisposable and in that dispose method you should call Dispose() on the object.

For event handlers, it is a matter of personal preference. You can, but it only matters if you do or not if the person who subscribed to the event messed up their code.

Unless you expect the publisher of the event to outlive the
subscriber, there's no reason to remove the event handler...

I personally do not, but I do know some people who do. If you wanted to do it, the process is simply setting the event handler to null in your dispose method.

public sealed class Example : IDisposable 
{
    public EventHandler MyEvent;

    public void Dispose()
    {
        MyEvent = null;
    }
}

---

EDIT: And a good point that Hans Passant brought up in the comments: You never need a finalizer, if you have a unmanaged resource that would need a finalizer it should get wrapped up in a SafeHandle wrapper to handle the finalization for you. Once you do that the object just becomes another normal IDisposable you need to take care of in your .Dispose() method.

Should I unsubscribe from events?

1) It depends. Usually it's a good idea, but there are typical cases where you don't need to. Basically, if you are sure that the subscribing object is going to outlive the event source, you ought to unsubscribe, otherwise this would create an unnecessary reference.

If however your object is subscribing to its own events, like in the following:

<Window Loaded="self_Loaded" ...>...</Window>

--then you don't have to.

2) Subscribing to an event makes additional reference to the subscribing object. So if you don't unsubscribe, your object might be kept alive by this reference, making effectively a memory leak. By unsubscribing you are removing that reference. Note that in the case of self-subscription the problem doesn't arise.

3) You can do like that:

this.PropertyChanged += PropertyChangedHandler;
...
this.PropertyChanged -= PropertyChangedHandler;

where

void PropertyChangedHandler(object o, PropertyChangedEventArgs e)
{
    switch (e.PropertyName)
    {
        case "FirstName": break;
        case "LastName": break;
    }
}

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

What's the good reason for removing an event after using it in javascript or unbinding after binding it in jquery?

The uses for this can be many and truly depends on your use case. Sometimes you only want an event to run once and you unbind after running your event listener finishes its function.

In certain apps such as a backbones app it comes down to performance. For example events that are bound to a view let's say, a view for an about page, will be persistant.

Let's say you want to switch between your about view to your "home" view. In backbone the events you define will still exist in memory even if you destroy the object they are bound to within the Dom. In the this case you will have a remove method, or something similar, that will unbind those events for your view object. This is extremely useful for a complex app. But these are only two examples. It's merely a tool and with that you can provide functionality for a large plethora of cases.

Do you need to remove an event handler in the destructor?

Since PPMM is a long-lived object (singleton), then this code doesn't make much sense.

The problem here is that as long as that event handler is referencing the object, it will not be eligible for garbage collection, as least as long as that other object that owns the event is alive.

As such, putting anything in the destructor is pointless, as either:

1. The event handler has already been removed, thus the object became eligible for garbage collection
2. The event handler is not removed, the owning object is not eligible for garbage collection, and thus the finalizer will never get called
3. Both objects are eligible for garbage collection, in which case you should not access that other object at all in the finalizer since you don't know its internal state

In short, don't do this.

Now, a different argument could be said about adding such code to the Dispose method, when you're implementing IDisposable. In that case it fully makes sense since its usercode that is calling Dispose, at a predefined and controlled point.

The finalizer (destructor), however, is only called when the object is eligible for garbage collection and has a finalizer, in which case there is no point.

As for question nbr. 2, which I take as "Can I unsubscribe from events like that", then yes, you can. The only time you need to hold on to the delegate you used to subscribe with is when you're constructing the delegate around an anonymous method or a lambda expression. When you're constructing it around an existing method, it will work.

---

Edit: WPF. right, didn't see that tag. Sorry, the rest of my answer doesn't make much sense for WPF and since I am no WPF-guru, I can't really say. However, there's a way to fix this. It's entirely legal here on SO to poach the content of another answer if you can improve it. So if anyone knows how to properly do this with a WPF usercontrol, you're free to lift the entire first section of my answer and add the relevant bits of WPF.

Edit: Let me respond to the question in the comment inside here as well.

Since the class in question is a user-control, its lifetime will be tied to a form. When the form is closing, it will dispose of all child controls that it owns, in other words, there is already a Dispose method present here.

The correct way for a user control to handle this, if it manages its own events, is to unhook the event handlers in the Dispose method.

(rest removed)

---

Related Topics