Dispose container when startable or build callback throws (#1392)

If StartStartableComponents or RunBuildCallbacks threw during Build(), the
newly created Container was abandoned without Dispose() being called.  Any
IDisposable instances that had been resolved into the root lifetime scope to
satisfy those operations were then leaked.

Wrap both calls in a try/catch that disposes the container and rethrows on
the failure path; on the success path the caller owns the container as before.

Also add a regression test that registers a DisposableDependency as a
transitive dependency of a throwing IStartable, asserts Build() throws, and
asserts the dependency was disposed.

Author: tillig

src/Autofac/ContainerBuilder.cs

@@ -175,13 +175,21 @@ public IContainer Build(ContainerBuildOptions options = ContainerBuildOptions.No
         var componentRegistry = ComponentRegistryBuilder.Build();
 
         var result = new Container(componentRegistry);
-        if ((options & ContainerBuildOptions.IgnoreStartableComponents) == ContainerBuildOptions.None)
+        try
         {
-            StartableManager.StartStartableComponents(Properties, result);
-        }
+            if ((options & ContainerBuildOptions.IgnoreStartableComponents) == ContainerBuildOptions.None)
+            {
+                StartableManager.StartStartableComponents(Properties, result);
+            }
 
-        // Run any build callbacks.
-        BuildCallbackManager.RunBuildCallbacks(result);
+            // Run any build callbacks.
+            BuildCallbackManager.RunBuildCallbacks(result);
+        }
+        catch
+        {
+            result.Dispose();
+            throw;
+        }
 
         // Allow the reflection cache to empty any registration-time caches to save memory.
         ReflectionCacheSet.Shared.OnContainerBuildClearCaches(_clearRegistrationCaches);

test/Autofac.Specification.Test/Features/StartableTests.cs

@@ -189,6 +189,23 @@ public void AutoActivate_RegisterInstanceActivationWorksWhenDefaultServiceOverlo
         Assert.Equal(1, instanceCount);
     }
 
+    [Fact]
+    public void Startable_WhenStartableThrows_DependenciesAreDisposed()
+    {
+        // Issue #1392 - disposable instances resolved to satisfy a startable should be
+        // disposed when the startable's Start() method throws.
+#pragma warning disable CA2000 // Dispose objects before losing scope - ownership transferred to container
+        var dependency = new DisposableDependency();
+#pragma warning restore CA2000 // Dispose objects before losing scope
+        var builder = new ContainerBuilder();
+        builder.RegisterInstance(dependency);
+        builder.RegisterType<ThrowingStartable>().As<IStartable>().SingleInstance();
+
+        Assert.Throws<DependencyResolutionException>(() => builder.Build());
+
+        Assert.True(dependency.IsDisposed);
+    }
+
     private class ComponentTakesStartableDependency : IStartable
     {
         public ComponentTakesStartableDependency(StartableTakesDependency dependency, bool expectStarted)
@@ -285,4 +302,29 @@ public void Start()
             WasStarted = true;
         }
     }
+
+    private class DisposableDependency : IDisposable
+    {
+        public bool IsDisposed
+        {
+            get; private set;
+        }
+
+        public void Dispose()
+        {
+            IsDisposed = true;
+        }
+    }
+
+    private class ThrowingStartable : IStartable
+    {
+        public ThrowingStartable(DisposableDependency dependency)
+        {
+        }
+
+        public void Start()
+        {
+            throw new InvalidOperationException("Startable failed.");
+        }
+    }
 }