CODEWITHSUNDEEP
c#asp.neteventsmemory-leaksevent-handling
sm_
sm_

Reputation: 2602

Removing event-handlers in destructors in case both publisher and subscriber hold reference to each other

Lets say we have the following model:

    public class Bar<T>:List<T>
    {
        public delegate void CollectionChangedDelegate();
        public event CollectionChangedDelegate CollectionChanged;
    }

    public class Foo
    {
        Bar<object> MyCollection = new Bar<object>();

        public Foo()
        {
            MyCollection.CollectionChanged += new Bar<object>.CollectionChangedDelegate(MyCollection_CollectionChanged);
        }

        public void MyCollection_CollectionChanged()
        {
            //Do Something
        }

        ~Foo() //Would this work ?
        {
            MyCollection.CollectionChanged -= MyCollection_CollectionChanged;
        }
    }

Could the destructor of Class Foo be called in this case ?

Upvotes: 3

Views: 1391

Answers (2)

Amir Ofir
Amir Ofir

Reputation: 136

first of all, I believe that you are trying to write ObservableCollection
You can find info here: http://msdn.microsoft.com/en-us/library/ms668604.aspx.

Now, if my object holds data-members that have a need to dispose, I would implement IDisposable and there I would dispose them, or at least remove the event subscription.
implment it like that: 

class Foo:Idisposable 
{
    public void Dispose(bool b)
    {
         MyCollection.CollectionChanged -= MyCollection_CollectionChanged;
    }
}

Upvotes: 3

Marc Gravell
Marc Gravell

Reputation: 1062780

It is pointless to do so. Let's assume that there is such a subscription. Now, for us to get to the ~Foo method we must be being collected, so we must be unreachable. Because of how events work, subscriptions make us reachable (the publisher has a reference to the subscriber) - so we can deduce that MyCollection is also unreachable. If it wasn't, we wouldn't be being collected.

And if MyCollection is unreachable, then either it has already been collected, or it is about to be collected. There is no need to unsubscribe.

Remove ~Foo: it is pointless here; actually, it is worse than pointless - in addition to serving no useful purpose, it forces the garbage collector to push this object through an extra step (the finalizer queue).

You might, however, want to add some kind of deterministic clean-up that does this; as Joroen notes, IDisposable may be of use - however, it is hard to say whether it is appropriate in the case without knowing more context.

Upvotes: 4

Related Questions