Constructor Exceptions in dispposable object

Question

Consider folowing classes:

class C1 : IDisposable {...}

class C2 : IDisposable {...}

sealed class C3 : IDisposable
{
   public C3()
   {
      c1 = new C1();
      throw new Exception(); //oops!
   }
   ~C3()
   {
      //What can we do???
   }
   public void Dispose()
   {
      if ( c1 != null ) c1.Dispose();
      if ( c2 != null ) c2.Dispose();
   }

   private C1 c1;
   private C2 c2;
   //assume that this class does not contains native resources
}

Now, assume we correctly use disposable object:

using (var c3 = new C3())
{
}

What about this code snippet?

In this case we can't call Dispose method, because our object never exists.

We know, that in this case finalizer would be called, but there we can dispose only CriticalFinilizedObjects and we can't dispose C1 or C2 objects.

My solution is pretty simple:

sealed class C3 : IDisposable
{
   public C3()
   {
      try {
      c1 = new C1();
      throw new Exception(); //oops!
      c2 = new C2();
      }
      catch(Exception)
      {
        DisposeImpl();
        throw;
      }
   }
   ~C3()
   {
      //Not deterministically dispose detected!
      //write to log! 
      //Invalid class usage. Or not??
   }
   public void Dispose()
   {
      DisposeImpl();
   }
   private void DisposeImpl()
   {
     if ( c1 != null ) c1.Dispose();
     if ( c2 != null ) c2.Dispose();
     GC.SuppressFinalize(this); //all resources are released
   }

   private C1 c1;
   private C2 c2;
}

This solution may vary in some details but I think you can understand key principle: if constructor throws an exception we forcedly release acquired resources and suppress finalization.

Have any one other idias, suggestions or better solutions?

P.S. Herb Sutter in his blogpost (http://herbsutter.wordpress.com/2008/07/25/constructor-exceptions-in-c-c-and-java/) opened the question but he does not proposed a solution.

Answer 1

What you're proposing is the conclusion I came to as well when thinking about this issue.

In summary, do as much work as needed to get the object in a usable state in the constructor, but clean up any expensive managed resources you allocate in an exception handler if you can't complete it successfully.

It is idiomatic in .NET to use the constructor to get the object in a state where it can be used. Some people will suggest the use of a simple constructor followed by an Initialize method where any 'real' work is done to get the object in the right state, but I cannot think of a single framework class that does this, so it's not an intuitive pattern for .NET developers to follow, and thus should not be done in .NET irrespective of whether it is a reasonable convention in other languages and platforms.

I think it's a fairly rare case anyway - typically a class that wraps a disposable resource will take it as a constructor parameter rather than creating it itself.

Answer 2

Isnt your dispose in actuality your deconstructor?

Are you trying to basically use RAII?

My program in its entirety

using System;

namespace Testing
{
    class C1 : IDisposable
    {
        public C1()
        {
        }
        public void Dispose()
        {
            Console.WriteLine( "C1 Destroyed" );
        }
    }
    class C2 : IDisposable
    {
        public C2()
        {
            throw new Exception();
        }
        public void Dispose()
        {
            Console.WriteLine( "C2 Destroyed" );
        }
    }
    class C3 : IDisposable
    {
        C1 c1;
        C2 c2;
        public C3()
        {
            try {
                c1 = new C1();
                c2 = new C2();
            } catch {
                this.Dispose();
                throw new Exception();
            }
        }
        ~C3()
        {
            this.Dispose();

        }
        public void Dispose()
        {
            // basically an early deconstructor
            Console.WriteLine( "C3 Being Destroyed" );
            if ( c1 != null )
                c1.Dispose();
            if ( c2 != null )
                c2.Dispose();
            GC.SuppressFinalize(this);
            Console.WriteLine( "C3 Destroyed" );
        }
    }
    class MainClass
    {
        public static void Main(string[] args)
        {
            try {
                using ( var c3 = new C3() )
                {
                    Console.WriteLine("Rawr");
                }
            } catch {
                Console.WriteLine( "C3 Failed" );
            }
            GC.Collect();
        }
    }
}

Answer 3

Do asserts on your constructor arguments as the very first. Assert that any subsequent logic will not throw an exception. That is assert that any data used in the constructor logic is valid for that specific usage. Like so:

c3(string someString)
{
  Debug.Assert(!string.IsNullOrEmpty())
  c1 = new c1(someString);
}

in the case where an empty string would cause c1 throw an exception.

If you are not able to ensure that no exceptions will be throw by input validation. Rewrite the code so you can. Since that would be a very strong smell. If the exception throwing code is not users but a vendor change vendor. It will not be the last problem you get with their code then.

Answer 4

Wrapping the body of the constructor in a "try" and calling Dispose on failure is probably about as good a thing as you can do. Instead of "Try-Catch", though, I'd suggest "Try-Finally", with an "ok" flag which gets set at the end of the "try". If something throws during your constructor, that will give the debugger a chance to look at the system state at the time of the throw.

In vb.net, one can do things a little better, since vb.net supports exception filtering, and since vb.net runs field initializers after the base-class constructor, and thus allows base-class members to be used within the derived-class field initializers.

See my question Handling iDisposable in failed initializer or constructor along with my answer to it for more info.

Answer 5

A better solution would be not to do ANY logic in constructors. Just create the object and it's good to go. If you really need to do something in the constructor, encapsulate it with a try catch finally statement where you free up unmanaged resources.