Implementing IDisposable on a sealed class

Question

I don't think this question has been asked before. I'm a bit confused on the best way to implement IDisposable on a sealed class—specifically, a sealed class that does not inherit from a base class. (That is, a "pure sealed class" which is my made up term.)

Perhaps some of you agree with me in that the guidelines for implementing IDisposable are very confusing. That said, I want to know that the way I intend to implement IDisposable is sufficient and safe.

I'm doing some P/Invoke code that allocates an IntPtr through Marshal.AllocHGlobal and naturally, I want to cleanly dispose of the unmanaged memory I've created. So I'm thinking of something like this

using System.Runtime.InteropServices;

[StructLayout(LayoutKind.Sequential)]
public sealed class MemBlock : IDisposable
{
     IntPtr ptr;
     int length;

     MemBlock(int size)
     {
           ptr = Marshal.AllocHGlobal(size);
           length = size;
     }

     public void Dispose()
     {
          if (ptr != IntPtr.Zero)
          {
               Marshal.FreeHGlobal(ptr);
               ptr = IntPtr.Zero;
               GC.SuppressFinalize(this);
          }
     }

     ~MemBlock()
     {
           Dispose();
     }    
}

I'm assuming that because MemBlock is completely sealed and never derives from another class that implementing a virtual protected Dispose(bool disposing) is not necessary.

Also, is the finalizer strictly necessary? All thoughts welcome.

score 15 · Accepted Answer · answered Sep 13 '09 at 22:06

15

The finalizer is necessary as a fallback mechanism to eventually free unmanaged resources if you forgot to call Dispose.

No, you shouldn't declare a virtual method in a sealed class. It wouldn't compile at all. Also, it's not recommended to declare new protected members in sealed classes.

answered Sep 13 '09 at 22:06

Mehrdad Afshari

414,610
91
852
789

But of course in the case that a sealed class derived from a base class then a virtual Dispose would be necessary - correct? – zebrabox Sep 13 '09 at 22:12
Also. The finaliser means adding to a finalizer queue and having the overhead of effectively a double garbage collect. Seems a heavy penalty to pay for using un-managed resources. Is there no way to avoid the performance hit? – zebrabox Sep 13 '09 at 22:18
In that case, you'd `override` the method. You can't declare any methods in a `sealed` class as `virtual`. It's a compiler **error**. – Mehrdad Afshari Sep 13 '09 at 22:18
3

`GC.SuppressFinalize` prevents finalization and its overheads. – Mehrdad Afshari Sep 13 '09 at 22:19
Yep my bad. I'm confusing override with virtual. You're right of course that a sealed class wouldn't allow a virtual in the first place! – zebrabox Sep 13 '09 at 22:20

score 11 · Answer 2 · answered Sep 13 '09 at 22:16

11

A minor addition; in the general case, a common pattern is to have a Dispose(bool disposing) method, so that you know whether you are in Dispose (where more things are available) vs the finalizer (where you shouldn't really touch any other connected managed objects).

For example:

 public void Dispose() { Dispose(true); }
 ~MemBlock() { Dispose(false); }
 void Dispose(bool disposing) { // would be protected virtual if not sealed 
     if(disposing) { // only run this logic when Dispose is called
         GC.SuppressFinalize(this);
         // and anything else that touches managed objects
     }
     if (ptr != IntPtr.Zero) {
          Marshal.FreeHGlobal(ptr);
          ptr = IntPtr.Zero;
     }
 }

answered Sep 13 '09 at 22:16

Marc Gravell

1,026,079
266
2,566
2,900

Yep good point Marc but if I knew that I was only disposing of unmanaged resources then is that strictly necessary? – zebrabox Sep 13 '09 at 22:22
Also - very stupid question but if the Dispose pattern is to deterministically release unmanaged resources then why would I want to handle disposable of managed resources when they should be cleaned up by the GC? – zebrabox Sep 13 '09 at 22:29
If you are being deterministic, then you would want to clean up anything that you are *encapsulating*; especially if they are themselves `IDisposable`. You *wouldn't* do this in the finalizer, as they may already have been collected (and: it isn't your job any longer). And you're right; in this case, other than the `SuppressFinalize` (which doesn't matter hugely) we're not doing anything managed, so it would be fine not to bother; which is why I emphasised the *general* case. – Marc Gravell Sep 13 '09 at 22:36
@zebrabox - Marc's right in general, but you have a very specific edge case. As to the why, your class might own managed resources that implement IDisposable. You then want to dispose of those when disposing manually but not in the finalizer. Read Joe Duffy's article (see the link in my answer) for much more. – TrueWill Sep 13 '09 at 22:38
@TrueWill and @Marc Gravell .Of course! ( slaps head in a semi-comedic fashion) makes sense now. Thanks both :) – zebrabox Sep 13 '09 at 22:46

score 8 · Answer 3 · edited Sep 26 '19 at 17:00

8

From Joe Duffy's Weblog:

For sealed classes, this pattern need not be followed, meaning you should simply implement your Finalizer and Dispose with the simple methods (i.e. ~T() (Finalize) and Dispose() in C#). When choosing the latter route, your code should still adhere to the guidelines below regarding implementation of finalization and dispose logic.

So yes, you should be good.

You do need the finalizer as Mehrdad mentioned. If you want to avoid it, you might take a look at SafeHandle. I don't have enough experience with P/Invoke to suggest the correct usage.

edited Sep 26 '19 at 17:00

JackMorrissey

2,567
2
21
18

answered Sep 13 '09 at 22:29

TrueWill

25,132
10
101
150

Thanks TrueWill! I have looked at SafeHandle and according to Eric Lippert it was viewed by the BCL team as one of the most significant benefits they introduced in 'Whidbey' ( sorry can't find the link for now ). Unfortunately it's an abstract class so you have to roll your own for every situation which kinda sucks – zebrabox Sep 13 '09 at 22:35
1

@zebrabox: While you might need to roll your own in some situations, the documentation states: "A set of prewritten classes derived from SafeHandle is provided as abstract derivations, and this set is located in the Microsoft.Win32.SafeHandles namespace." – TrueWill Sep 13 '09 at 22:46
1

@TrueWill. Yep very true but only for stuff like File Handles, Wait Handles, Pipe Handles and a bunch of of crypt stuff. Still better than nothing! – zebrabox Sep 13 '09 at 22:54
1

@zebrabox: Good point. It looks like it's not that difficult to create a descendant, and you only need to do it once per handle type/scenario. I found a nice article at http://blogs.msdn.com/shawnfa/archive/2004/08/12/213808.aspx - it's pre-release, so you may want to verify the information against the MSDN docs. – TrueWill Sep 14 '09 at 00:49
@TrueWill. Thanks for the link. Will check it out further – zebrabox Sep 14 '09 at 08:42

score 1 · Answer 4 · answered Sep 13 '09 at 23:12

You cannot declare virtual methods in a sealed class. Also declaring protected members in a sealed class gives you a compiler warning. So you've implemented it correctly. Calling GC.SuppressFinalize(this) from within the finalizer is not necessary for obvious reasons but it cannot harm.

Having a finalizer is essential when dealing with unmanaged resources, because they are not freed automatically, you have to do it in the finalizer with is called automatically after the object has been garbage collected.

Implementing IDisposable on a sealed class

4 Answers4

Linked