What is the correct way to implement the IDisposable-Pattern, given that the implementing class contains a GCHandle-Member?

I came up with this, but it causes my application to leak memory:

public class foo : IDisposable
{
     private bool disposed = false;
     private readonly byte[] bytes;
     private readonly GCHandle hBytes;
     private readonly IDisposable someWrapperForUnmanagedData;

     public foo(byte[] bytes)
     {
          this.bytes = bytes;
          hBytes = GCHandle.Alloc(bytes, GCHandleType.Pinned);

          someWrapperForUnmanagedData = new bar(..., bytes, ...);
     }

     protected virtual void Dispose(bool disposing)
     {
          if (disposed) return;
          if (disposing)
          {
               hBytes.Free();
          }
          someWrapperForUnmanagedData.Dispose();
          disposed = true;
     }

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

     ~foo() => Dispose(false);
}

Microsoft-documentation does not mention how GCHandles are supposed to be used with IDisposable, and I can't seem to find anything on the net.

Should the finalizer for foo call hBytes.Free()? Following the documentation, the finalizer is only supposed to call cleanup-routines for unmanaged resources, which in this case is only someWrapperForUnmanagedData.

CodePudding user response：

You've got your Dispose method backwards.

1. When your type's finalizer is called, you should release managed resources that you own.
2. When your type's IDisposable.Dispose() method is called, you should call Dispose on the things you own, and also release managed resources that you own.

The reason for this is straightforward. The happy case is that your type is disposed (someone calls .Dispose() on it). This gives you the opportunity to release unmanaged resources that you own, and you also need to propagate this Dispose() call down to your children so that they can release their unmanaged resources as well.

If someone forgets to call .Dispose() on you, then the GC will probably save your back by calling your finalize method. In this case, you should release any unmanaged resources you own, but you should not call into any of your children. If any of your children have their own finalizers, then:

1. The GC will call their finalizer separately, so you should not call it as well.
2. There's no guarantee of the order that types are finalized, so your children might already have been finalized.

Note the use of the term ownership. An unmanaged resource should have exactly one thing which owns it, and that is the thing which is responsible for releasing it.

Note that your someWrapperForUnmanagedData is not an unmanaged type. It's a C# class, which is very much managed. Unmanaged types are things like raw pointers. If someWrapperForUnmanagedData itself owns some other unmanaged type(s), it should implemenet IDisposable and define a finalizer in order to make sure that those are freed.

GCHandle doesn't have its own finalizer, so there's no way for it to be Free()ed by the GC. Therefore it counts as an unmanaged resource here: you'll need to manually call Free() on it.

So:

     protected virtual void Dispose(bool disposing)
     {
          if (disposed) return;

          if (disposing)
          {
               someWrapperForUnmanagedData.Dispose();
          }
          hBytes.Free();
          
          disposed = true;
     }

Note, as this doc makes clear, you should use a SafeHandle (or one of its subclasses) if possible. SafeHandle implements its own finalizer, so you don't have to.

The runtime also knows about SafeHandle, and this lets it avoid some really nasty race conditions where e.g. your type can be finalized at the same time as it is making an unmanaged call, leading to a nasty crash. If you don't understand this race, this means you should definitely be using a SafeHandle.