A call to the Dispose method inside the constructor that throws an exception or behaves unexpectedly

Question

I have a class that consumes some unmanaged resources, and I would like to free them deterministically and require that the finalizer not be called for the object at hand. My class's Dispose() method does this.

In the case of an exception being thrown or something else that is going wrong in the constructor, I would like to call Dispose() before throwing. However, I rarely come across implementations that catch abandoned exceptions or handle an error in the constructor of one-time objects, and then call Dispose() on the object. In many cases, the author leaves the cleanup in the finalizer. I have not read anything that suggests that calling Dispose() on a failed constructor is bad practice, but looking through the .NET source code, I still have to encounter such an exception or error handling in a one-time object constructor.

Is it possible to call Dispose() inside the constructor "crash" and still be considered a good coding citizen?

Edit to clarify . I am talking about inside the constructor:

 public class MyClass : IDisposable { private IntPtr _libPtr = IntPtr.Zero; public MyClass(string dllPath) { _libPtr = NativeMethods.LoadLibrary(dllPath); if (_libPtr != IntPtr.Zero) { IntPtr fxnPtr = NativeMethods.GetProcAddress(_libPtr, "MyFunction"); if (fxnPtr == IntPtr.Zero) { Dispose(); // Cleanup resources - NativeMethods.FreeLibrary(_libPtr); throw new NullReferenceException("Error linking library."); } } else { throw new DllNotFoundException("Something helpful"); } } // ... } 

Answer 1

I would not have to call the Dispose object for myself, but if necessary I would have a constructor. I would also like to make this cleaning as simple as possible. Given your example, I would rather write something like:

 internal sealed class Library : IDisposable { IntPtr _libPtr; // Or better yet, can we use or derive from SafeHandle? public Library(string dllPath) { _libPtr = NativeMethods.LoadLibrary(dllPath); if(_libPtr == IntPtr.Zero) { GC.SuppressFinalize(this); throw new DllNotFoundException("Library Load Failed"); } } private void Release() { if(_libPtr != IntPtr.Zero) NativeMethods.FreeLibrary(_libPtr); _libPtr = IntPtr.Zero; // avoid double free even if a caller double-disposes. } public void Dispose() { Release(); GC.SuppressFinalize(this); } ~Library() { Release(); } public IntPtr GetProcAddress(string functionName) { if(_libPtr == IntPtr.Zero) throw new ObjectDisposedException(); IntPtr funcPtr = NativeMethods.GetProcAddress(_libPtr, functionName); if(_funcPtr == IntPtr.Zero) throw new Exception("Error binding function."); return _funcPtr; } } 

It is still nice and simple. Either this object was successfully created and can be released by the code that called it, or it does not need to be cleaned. We can even prevent the finalization of no-op, just to be enjoyable. The main thing is that there is nothing that needs to be cleaned up, created after the last thing that could reasonably go wrong.

And then:

 public sealed class MyClass : IDisposable { private readonly Library _lib; private readonly IntPtr _funcPtr; public MyClass(string dllPath) { _lib = new Library(dllPath); // If this fails, we throw here, and we don't need clean-up. try { _funcPtr = _libPtr.GetProcAddress("MyFunction"); } catch { // To be here, _lib must be valid, but we've failed over-all. _lib.Dispose(); throw; } } public void Dispose() { _lib.Dispose(); } // No finaliser needed, because no unmanaged resources needing finalisation are directly held. } 

Again, I can provide cleanup, but I do not call this.Dispose(); While this.Dispose() can do the same trick, I prefer to have the field that I clear explicitly in the same method (constructor here) that installed it but failed to do all of its work. Firstly, the only place where a partially constructed object can be is the constructor, so the only place I need to consider a partially constructed object is in the constructor; I made it the invariant of the rest of the class that _lib not null.

Suppose functions were to be released separately from libraries, just to have a more complex example. Then I would also _funcPtr to follow a simplifying rule; either the class has one unmanaged resource that it cleans up with Dispose() and a finalizer, or it has one or more IDisposable fields that it cleans up with Dispose or it doesn’t need to be deleted, but it never combines above.

 internal sealed class Function : IDisposable { IntPtr _funcPtr; // Again better yet, can we use or derive from SafeHandle? public Function(Lib library, string functionName) { _funcPtr = library.GetProcAddress(functionName); if(_funcPtr == IntPtr.Zero) { GC.SuppressFinalize(this); throw new Exception("Error binding function."); } } private void Release() { if(_funcPtr != IntPtr.Zero) NativeMethods.HypotheticalForgetProcAddressMethod(_funcPtr); _funcPtr = IntPtr.Zero; // avoid double free. } public void Dispose() { Release(); GC.SuppressFinalize(this); } ~Function() { Release(); } } 

And then MyClass will be:

 public sealed class MyClass : IDisposable { private Library _lib; private Function _func; public MyClass(string dllPath) { _lib = new Library(dllPath); // If this fails, we throw here, and we don't need clean-up. try { _func = new Function(_lib, "MyFunction"); try { SomeMethodThatCanThrowJustToComplicateThings(); } catch { _func.Dispose(); throw; } } catch { _lib.Dispose(); throw; } } public void Dispose() { _func.Dispose(); _lib.Dispose(); } } 

This makes the constructor a bit more verbose, and I would rather avoid two things that might go wrong to affect the two things that need to be cleaned up first. It, although it reflects why I like cleaning, to be explicit in relation to different fields; Perhaps I want to clear both fields or only one, depending on where the exception will occur.

Answer 2

What you are describing is a template implemented by the C ++ / CLI compiler, which should be standard for all .NET languages, but it is not. The inability of .NET to indicate that a failed constructor should cause Dispose to be Dispose on a partially constructed object (and that any legitimate Dispose implementation must be prepared to solve this problem) means that many kinds of objects must or require the use of factory methods rather than constructors , require an inconvenient two-stage construction sequence in which the objects are in a strange "uncertainty" until the second stage is completed, or accepts a "hope that is not too wrong" for processing errors and cleaning.

Of these approaches, it is best to probably require using the factory method to build. Since factory methods are specific to the type of object being created, this approach requires derived classes to include some annoying patterns in the action:

 DerivedFoo Create(params) { // Phase 1 shouldn't allocate resources yet Derived foo result = new DerivedFoo(params); // NON-VIRTUAL Phase 2 method which chains to a virtual one within try/finally result.Initialize(); return result; } 

Not entirely terrible, but annoying .. The .NET Framework could greatly benefit from providing classes with an Initialize method that will be called between the completion of the derived constructor and the return code to the client, but since such a function does not exist officially, it can do this. probably by hand. [I think there are some kludges designed for COM interoperability that may help, but I don't know how well they are supported].