How to Design an IDisposable That Needs to Be Disposed

cclass-designdesigngarbage-collection

Consider a class that implements IDisposable, and that has members in such a way that it will never become eligible for garbage collection when it is not disposed. And as it will not be garbage collected, it will not have the chance to use the destructor for cleaning up.

As a result, when it is not disposed (e.g. unreliable users, programming errors), resources will be leaked.

Is there a general approach how such a class can be designed to deal with such a situation or to avoid it?

Example:

using System;

class Program
{
    static void Main(string[] args)
    {
        new Cyclical();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Console.ReadKey();
    }
}

class Cyclical
{
    public Cyclical()
    {
        timer = new System.Threading.Timer(Callback, null, 0, 1000);
    }

    System.Threading.Timer timer;

    void Callback(object state)
    {
        Console.Write('.');  // do something useful
    }

    ~Cyclical()
    {
        Console.WriteLine("destructor");
    }
}

(Omitted IDisposable to keep example short.) This class uses a Timer to do something useful at certain intervals. It needs a reference to the Timer to avoid it is garbage collected.

Let’s assume the user of that class will not dispose it. As a result of the timer, somewhere some worker thread has a reference to the Cyclical instance via the callback, and as a result, the Cyclical instance will never become eligible for garbage collection, and its destructor will never run, and resources will leak.

In this example, a possible fix (or workaround) could be to use a helper class that receives callbacks from the Timer, and that does not have a reference, but only a WeakReference to the Cyclical instance, which it calls using that WeakReference.

However, in general, is there a design rule for classes like this that need to be disposed to avoid leaking resources?

For the sake of completeness, here the example including IDispose and including a workaround/solution (and with a hopefully less distracting name):

class SomethingWithTimer : IDisposable
{
   public SomethingWithTimer()
   {
      timer = new System.Threading.Timer(StaticCallback,
         new WeakReference<SomethingWithTimer>(this), 0, 1000);
   }

   System.Threading.Timer timer;

   static void StaticCallback(object state)
   {
      WeakReference<SomethingWithTimer> instanceRef
         = (WeakReference<SomethingWithTimer>) state;
      SomethingWithTimer instance;
      if (instanceRef.TryGetTarget(out instance))
         instance.Callback(null);
   }

   void Callback(object state)
   {
      Console.Write('.');  // do something useful
   }

   public void Dispose()
   {
      Dispose(true);
      GC.SuppressFinalize(this);
   }

   protected virtual void Dispose(bool disposing)
   {
      if (disposing)
      {
         Console.WriteLine("dispose");
         timer.Dispose();
      }
   }

   ~SomethingWithTimer()
   {
      Console.WriteLine("destructor");
      Dispose(false);
   }
}
  • If disposed, the timer will be disposed.
  • If not disposed, the object will become eligible for garbage collection.

Best Answer

One approach which can be helpful is to expose wrapper objects to the outside world, but don't hold any strong references to them internally. Either have the internal objects keep weak references to the wrappers, or else have each wrapper hold the only reference anywhere in the world to a finalizable object which in turn holds a reference to the "real" object [I don't like having any outside-world-facing objects implement Finalize, since objects have no control over who can call things like GC.SuppressFinalize() on them]. When all outside-world references to the wrapper object have disappeared, any weak references to the wrapper object will be invalidated [and can be recognized as such], and any finalizable object to which the wrapper held the only reference will run its Finalize method.

For event handlers which are guaranteed to be triggered on some period basis (e.g. timer ticks) I like the WeakReference approach. There's no need to use a finalizer to ensure the timer gets cleaned up; instead, the timer-tick event can notice that the outside-world link has been abandoned and clean itself up. Such an approach may also be workable for things whose only resources are subscriptions to rarely-fired events from long-lived (or static) objects, if those objects have a CheckSubscription event which fires periodically when a subscriber is added (the rate at which those events fire should depend upon the number of subscriptions). Event subscriptions pose a real problem is when an unbounded number of objects may subscribe to an event from a single instance of a long-lived object and be abandoned without unsubscribing; in that case, the number of abandoned-but-uncollectable subscriptions may grow without bound. If 100 instances of a class subscribe to an event from a long-lived object and are subsequently abandoned, and nothing else ever subscribes to that event, memory used by those subscriptions may never get cleaned up, but the quantity of wasted memory would be limited to those 100 subscriptions. Adding more subscriptions would at some point cause the CheckSubscription event to fire, which would in turn cause all the abandoned objects' subscriptions to get canceled.

PS--Another advantage of using weak references is that one can request that a WeakReference remain valid until the object to which it refers is 100% absolutely and irretrievably gone. It's possible for an object to appear abandoned and have its Finalize method scheduled, but then get resurrected and returned to active use even before its Finalize method actually executes; this can happen completely outside the object's control. If code holds a long weak reference to the public wrapper object, and if it makes sure the reference itself remains rooted, it can hold off on performing any cleanup until resurrection becomes impossible.

Related Topic