Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

.NET object events and dispose / GC

EDIT: After Joel Coehoorns excellent answer, I understand that I need to be more specific, so I modified my code to be closer to thing I'm trying to understand...

Events: As I understand, in the background the events are "collection" of EventHandlers aka Delegates which will be executed when event raised. So for me it means that if object Y has event E and object X subscribes to event Y.E, then Y will have reference to X, since Y must execute the method located in X, in that way, X can not be collected, and that thing i understand.

//Creates reference to this (b) in a.
a.EventHappened += new EventHandler(this.HandleEvent);

But it is not what Joel Coehoorn tells...

However, there is an issue with events such that sometimes people like to use IDisposable with types that have events. The problem is that when a type X subscribes to events in another type Y, X now has a reference to Y. This reference will prevent Y from being collected.

I not understand how X will reference the Y ???

I modified a bit my example to illustrate my case more closer:

class Service //Let's say it's windows service that must be 24/7 online
{       
    A _a;

    void Start()
    {
       CustomNotificationSystem.OnEventRaised += new EventHandler(CustomNotificationSystemHandler)
       _a = new A();

       B b1 = new B(_a);
       B b2 = new B(_a);
       C c1 = new C(_a);
       C c2 = new C(_a);
    }

    void CustomNotificationSystemHandler(args)
    {

        //_a.Dispose(); ADDED BY **EDIT 2***
        a.Dispose();

        _a = new A();
        /*
        b1,b2,c1,c2 will continue to exists as is, and I know they will now subscribed
        to previous instance of _a, and it's OK by me, BUT in that example, now, nobody
        references the previous instance of _a (b not holds reference to _a) and by my
        theory, previous instance of _a, now may be collected...or I'm missing
        something???
        */
    }

}  

class A : IDisposable
        {
           public event EventHandler EventHappened;
        }

        class B
        {          
           public B(A a) //Class B does not stores reference to a internally.
           {
              a.EventHappened += new EventHandler(this.HandleEventB);
           }

           public void HandleEventB(object sender, EventArgs args)
           {
           }
        }

        class C
        {          
           public C(A a) //Class B not stores reference to a internally.
           {
              a.EventHappened += new EventHandler(this.HandleEventC);
           }

           public void HandleEventC(object sender, EventArgs args)
           {
           }
        }

EDIT 2: OK, now it's clear, when subscriber subscribes to a publishers events, it's NOT creates a reference to the publisher in subscriber. Only the reference from publisher to subscriber created (through EventHandler)...in this case when publisher collected by GC before the subscriber (subscribers lifetime is greater then publishers), there's no problem.

BUT...as I know, it's not guaranteed when GC will collect the publisher so in theory, even if subscribers lifetime is greater then publishers, it can happen that subscriber is legal for collection, but publisher is still not collected (I don't know if within closest GC cycle, GC will be smart enough to collect publisher first and then subscriber.

Anyway, in such case, since my subscriber do not have direct reference to publisher and can't unsubscribe the event, I would like to make publisher to implement IDisposable, in order to dispose it before delete all references to him (see in CustomNotificationSystemHandler in my example).

AND AGAIN What I should write in publishers dispose method in order to clear all references to subscribers? should it be EventHappened -= null; or EventHappened = null; or there's no way to do it in such way, and I need to make something like below ???

public event EventHandler EventHappened
   {
      add 
      {
         eventTable["EventHappened"] = (EventHandler)eventTable["EventHappened"] + value;
      }
      remove
      {
         eventTable["EventHappened"] = (EventHandler)eventTable["EventHappened"] - value; 
      }
   }
like image 535
Alex Dn Avatar asked Jul 16 '12 15:07

Alex Dn


2 Answers

The life time of object B is longer that A, so A may be disposed earlier

It sounds like you are confusing "Disposal" with "Collection"? Disposing an object has nothing to do with memory or garbage collection. To make sure everything is clear, let's break up the two scenarios, and then I'll move on to events at the end:

Collection:

Nothing you do will ever allow A to be collected before it's parent B. As long as B is reachable, so is A. Even though A is private, it's still reachable from any code within B, and so as long as B is reachable, A is considered reachable. This means the garbage collector doesn't know for sure that you're done with it, and will never collect A until it is also safe to collect B. This is true even if you explicitly call GC.Collect() or similar. As long an object is reachable, it will not be collected.

Disposal:

I'm not even sure why you are implement IDisposable here (it has nothing to do with memory or garbage collection), but I'll give you the benefit of the doubt for the moment that we just don't see the unmanaged resource.

Nothing prevents you from Disposing A whenever you want. Just call a.Dispose(), and it's done. The only way the .Net framework will ever call Dispose() for you automatically is at the end of using block. Dispose() is not called during garbage collection, unless you do it as part of the object's finalizer (more on finalizers in a moment).

When implementing IDisposable, you are sending a message to programmers that this type should (maybe even "must") be disposed promptly. There are two correct patterns for any IDisposable object (with two variations on the pattern). The first pattern is to enclose the type itself in a using block. When this is not possible (for example: code such as yours where the type is a member of another type), the second pattern is that the parent type should also implement IDisposable so it can itself then be included in a using block, and it's Dispose() can call your type's Dispose(). The variation on these patterns is to use try/finally blocks instead of a using block, where you call Dispose() in the finally block.

Now on to finalizers. The only time you need to implement a finalizer is for an IDisposable type that originates an unmanaged resource. So, for example, if your type A above is just wrapping a class like SqlConnection, it does not need a finalizer because the finalizer in SqlConnection itself will take care of any needed cleanup. But, if your type A were implementing a connection to a whole new kind of database engine, you would want a finalizer to make sure your connections are closed when the object is collected. Your type B, however, would not need a finalizer, even though it manages/wraps your type A, because type A will take care of finalizing the connections.

Events:

Technically, events are still managed code and shouldn't need to be disposed. However, there is an issue with events such that sometimes people like to use IDisposable with types that have events. The problem is that when a type X subscribes to events in another type Y, Y now has a reference to X. This reference can prevent X from being collected. If you expected Y to have a longer lifetime then X, you can run into problems here, particularly if Y is very long-lived relative to many X's that come and go over time.

To get around this, sometimes programmers will have type Y implement IDisposable, and the purpose of the Dispose() method is to unsubscribe any events so that subscribing objects can also be collected. Technically, this is not the purpose of the Dispose() pattern, but it works well enough that I'm not going to argue about it. There are two things you need to know when using this pattern with events:

  1. You do not need a finalizer if this is the only reason for implementing IDisposable
  2. Instances of your type still need a using or try/finally block, or you haven't gained anything. Otherwise Dispose() will not be called and your objects still cannot be collected.

In this case, your type A is private to type B, and so only type B can subscribe to A's events. Since 'a' is a member of type B, neither is eligible for garbage collection until B is no longer reachable, at which point both will no longer be reachable and the event subscription reference won't count. That means a reference held on B by A's event would not prevent B from being collected. However, if you use the A type in other places, you may still want to have A implement IDisposable to make sure your events are unsubscribed. If you do that, make sure to follow the whole pattern, such that instances of A are enclosed in using or try/finally blocks.

like image 98
Joel Coehoorn Avatar answered Sep 24 '22 01:09

Joel Coehoorn


I have added My comments in your sample code.

class A : IDisposable
{
   public event EventHandler EventHappened
   {
      add 
      {
         eventTable["EventHappened"] = (EventHandler)eventTable["EventHappened"] + value;
      }
      remove
      {
         eventTable["EventHappened"] = (EventHandler)eventTable["EventHappened"] - value; 
      }
   }

   public void Dispose()
   {
      //Amit: If you have only one event 'EventHappened', 
      //you can clear up the subscribers as follows

      eventTable["EventHappened"] = null;

      //Amit: EventHappened = null will not work here as it is 
      //just a syntactical sugar to clear the compiler generated backing delegate.
      //Since you have added 'add' and 'remove' there is no compiler generated 
      //delegate to clear
      //
      //Above was just to explain the concept.
      //If eventTable is a dictionary of EventHandlers
      //You can simply call 'clear' on it.
      //This will work even if there are more events like EventHappened          
   }
}

class B
{          
   public B(A a)
   {
      a.EventHappened += new EventHandler(this.HandleEventB);

      //You are absolutely right here.
      //class B does not store any reference to A
      //Subscribing an event does not add any reference to publisher
      //Here all you are doing is calling 'Add' method of 'EventHappened'
      //passing it a delegate which holds a reference to B.
      //Hence there is a path from A to B but not reverse.
   }

   public void HandleEventB(object sender, EventArgs args)
   {
   }
}

class C
{          
   public C(A a)
   {
      a.EventHappened += new EventHandler(this.HandleEventC);
   }

   public void HandleEventC(object sender, EventArgs args)
   {
   }
}

class Service
{       
    A _a;

    void Start()
    {
       CustomNotificationSystem.OnEventRaised += new EventHandler(CustomNotificationSystemHandler)

       _a = new A();

       //Amit:You are right all these do not store any reference to _a
       B b1 = new B(_a);
       B b2 = new B(_a);
       C c1 = new C(_a);
       C c2 = new C(_a);
    }

    void CustomNotificationSystemHandler(args)
    {

        //Amit: You decide that _a has lived its life and must be disposed.
        //Here I assume you want to dispose so that it stops firing its events
        //More on this later
        _a.Dispose();

        //Amit: Now _a points to a brand new A and hence previous instance 
        //is eligible for collection since there are no active references to 
        //previous _a now
        _a = new A();
    }    
}

b1,b2,c1,c2 will continue to exists as is, and I know they will now subscribed to previous instance of _a, and it's OK by me, BUT in that example, now, nobody references the previous instance of _a (b not holds reference to _a) and by my theory, previous instance of _a, now may be collected...or I'm missing something???

As explained through my comments in the above code, you are not missing anything here :)

BUT...as I know, it's not guaranteed when GC will collect the publisher so in theory, even if subscribers lifetime is greater then publishers, it can happen that subscriber is legal for collection, but publisher is still not collected (I don't know if within closest GC cycle, GC will be smart enough to collect publisher first and then subscriber.

Since publisher references subscriber, it can never happen that the subscriber becomes eligible for collection before the publisher but reverse can be true. If publisher gets collected before subscriber then, as you said, there is no problem. If the subscriber belongs to a lower GC generation than publisher then since publisher holds a reference to subscriber, GC will treat the subscriber as reachable and will not collect it. If both belong to same generation, they will be collected together.

since my subscriber do not have direct reference to publisher and can't unsubscribe the event, I would like to make publisher to implement IDisposable

Contrary to what some have suggested, I would recommend implementing dispose if at any point you are deterministically sure that the object is no longer required. Simply updating an object reference may not always lead to an object stop publishing events.

Consider the following code:

class MainClass
{
    public static Publisher Publisher;

    static void Main()
    {
        Publisher = new Publisher();

        Thread eventThread = new Thread(DoWork);
        eventThread.Start();

        Publisher.StartPublishing(); //Keep on firing events
    }

    static void DoWork()
    {
        var subscriber = new Subscriber();
        subscriber = null; 
        //Subscriber is referenced by publisher's SomeEvent only
        Thread.Sleep(200);
        //We have waited enough, we don't require the Publisher now
        Publisher = null;
        GC.Collect();
        //Even after GC.Collect, publisher is not collected even when we have set Publisher to null
        //This is because 'StartPublishing' method is under execution at this point of time
        //which means it is implicitly reachable from Main Thread's stack (through 'this' pointer)
        //This also means that subscriber remain alive
        //Even when we intended the Publisher to stop publishing, it will keep firing events due to somewhat 'hidden' reference to it from Main Thread!!!!
    }
}

internal class Publisher
{
    public void StartPublishing()
    {
        Thread.Sleep(100);
        InvokeSomeEvent(null);
        Thread.Sleep(100);
        InvokeSomeEvent(null);
        Thread.Sleep(100);
        InvokeSomeEvent(null);
        Thread.Sleep(100);
        InvokeSomeEvent(null);
    }

    public event EventHandler SomeEvent;

    public void InvokeSomeEvent(object e)
    {
        EventHandler handler = SomeEvent;
        if (handler != null)
        {
            handler(this, null);
        }
    }

    ~Publisher()
    {
        Console.WriteLine("I am never Printed");
    }
}

internal class Subscriber
{
    public Subscriber()
    {
        if(MainClass.Publisher != null)
        {
            MainClass.Publisher.SomeEvent += PublisherSomeEvent;
        }
    }

    void PublisherSomeEvent(object sender, EventArgs e)
    {
        if (MainClass.Publisher == null)
        {
            //How can null fire an event!!! Raise Exception
            throw new Exception("Booooooooommmm");
            //But notice 'sender' is not null
        }
    }
}

If you run the above code, more often than not you will receive the 'Booooooooommmm'. Hence idea is that event publisher must stop firing events when we are sure that its life is up.

This can be done through Dispose method.

There are two ways to achieve this:

  1. Set a flag 'IsDisposed' and check it before firing any event.
  2. Clear up the event subscribers list (as suggested in my comments in your code).

Benefit of 2 is that you release any reference to the subscribers, thereby enabling there collection (as I explained earlier even if the publisher is garbage but belongs to higher generation then it may still prolong collection of lower generation subscribers).

Though, admittedly, it will be quite rare that you experience the demonstrated behavior due to 'hidden' reachability of the publisher but as you can see benefits of 2 are clear and are valid for all event publishers especially long living ones (Singletons anybody!!!). This itself makes it worth to implement Dispose and go with 2.

like image 31
Amit Mittal Avatar answered Sep 25 '22 01:09

Amit Mittal