Implementing C# IDisposable

Question

I am trying to work out exactly what I need to do to dispose of a property in C# when using IDisposable. I can see the template, but I am not sure about actually disposing of objects.

I am working with the example given on this site: https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose

I have added 3 properties:

privInt: Private value type. Would I dispose of this as it could be saved on the heap so may require GC? If I would then how, cannot set to null or call dispose? If not then I assume it has to be left to GC - but is it not the point of Dispose to free resources?

NonIDisClassInstance - Do I set to null? Is that enough

Can anyone comment on my implementation below and advise on what is correct/wrong.

IDisClass - Just call dispose

class BaseClass : IDisposable
{
   // Flag: Has Dispose already been called?
   bool disposed = false;

   // Public implementation of Dispose pattern callable by consumers.
   public void Dispose()
   { 
      Dispose(true);
      GC.SuppressFinalize(this);           
   }

   //Private value type
   private int privInt;

   //Private class that does not implement IDisposable
   Private NonIDisClass NonIDisClassInstance;

   //Private class that does implement IDisposable
   Private IDisClass IDisClassInstance;

   // Protected implementation of Dispose pattern.
   protected virtual void Dispose(bool disposing)
   {
      if (disposed)
         return; 

      if (disposing) {
         // Free any other managed objects here.
         //
      }

      // Free any unmanaged objects here.     
      DisposeOfThis = ????;
      NonIDisClassInstance = null;
      IDisClassInstance.Dispose();
      disposed = true;
   }

   ~BaseClass()
   {
      Dispose(false);
   }
}

You'll probably have better luck at https://codereview.stackexchange.com/ — asawyer, Nov 22 '19 at 21:23
If you are using a *destructor/finalizer* you are most likely doing something wrong to start with — TheGeneral, Nov 22 '19 at 21:32
The point of dispose is to free _unamanaged_ resources in a timely manner. Managed resources will be taken care of by the GC. Unmanaged resources would be, too, but using disposable releases them when you're done with them rather than waiting for the GC. You have no unmanaged resources, but do have a disposable object, so it should be disposed of in the `Dispose` method. — D Stanley, Nov 22 '19 at 21:38
Your implementation is subtly wrong as it tries to access *other* managed objects in finalizer (and there is no indication that it even has any unmanaged resource). — Alexei Levenkov, Nov 22 '19 at 21:40
@asawyer CR is for code that works correctly - this is not the case here and it is exactly what OP is asking. — Alexei Levenkov, Nov 22 '19 at 22:01

score 2 · Accepted Answer · edited Dec 15 '19 at 04:31

You only need GC.SuppressFinalize(this); and a destructor if you are working with unmanaged memory. If not you can safely omit this.
In your Dispose implementation call this.IDisClassInstance?.Dispose();, that is all you need.

Your code can be condensed down to this.

class BaseClass : System.IDisposable
{

    public virtual void Dispose()
    {
        // never throw an exception from Dispose so prevent an NRE by using ?.
        this.IDisClassInstance?.Dispose();
    }

    //Private class that does not implement IDisposable
    private object NonIDisClassInstance;

    //Private class that does implement IDisposable
    private IDisposable IDisClassInstance;
}

Do keep in mind that using Dispose is not necessarily about freeing up memory, it is about cleaning up / freeing up unmanaged resources which might also include memory. Things like open file streams or database connections or network connections.

Note that your derived types can override Dispose but those overriden methods should include a call to base.Dispose(); so the managed resources of the base class are released.

This implementation is wrong because it does not allow derived classes to *always* release unmanaged resources and even if it is just this class that happen to .have unmanaged resource it will fail to release it if `Dispose` not called. Consider explaining that as part of the answer. — Alexei Levenkov, Nov 22 '19 at 22:01
@AlexeiLevenkov - corrected, that method should have been marked as `virtual` seeing as the name of the type is `BaseClass` and is unlikely to be sealed. — Igor, Nov 22 '19 at 22:08

Implementing C# IDisposable

1 Answers1