Merge pull request #2654 from arturosevilla/assembly-catalog-unloads-after-detection

AppDomainAssemblyCatalog correctly unloads the assemblies that were probed for any Nancy reference.

Author: thecodejunkie

src/Nancy.MSBuild/Nancy.csproj

@@ -357,6 +357,9 @@
     <Compile Include="..\Nancy\Helpers\ExceptionExtensions.cs">
       <Link>Helpers\ExceptionExtensions.cs</Link>
     </Compile>
+    <Compile Include="..\Nancy\Helpers\ProxyNancyReferenceProber.cs">
+      <Link>Helpers\ProxyNancyReferenceProber.cs</Link>
+    </Compile>
     <Compile Include="..\Nancy\HttpLink.cs">
       <Link>HttpLink.cs</Link>
     </Compile>

src/Nancy/AppDomainAssemblyCatalog.cs

@@ -6,6 +6,8 @@ namespace Nancy
     using System.IO;
     using System.Linq;
     using System.Reflection;
+    using Nancy.Extensions;
+    using Nancy.Helpers;
 
     /// <summary>
     /// Default implementation of the <see cref="IAssemblyCatalog"/> interface, based on
@@ -40,7 +42,7 @@ private static List<Assembly> GetLoadedNancyReferencingAssemblies()
 
             foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies())
             {
-                if (!assembly.IsDynamic && !assembly.ReflectionOnly && IsNancyReferencing(assembly))
+                if (!assembly.IsDynamic && !assembly.ReflectionOnly && assembly.IsReferencing(NancyAssemblyName))
                 {
                     assemblies.Add(assembly);
                 }
@@ -52,7 +54,8 @@ private static List<Assembly> GetLoadedNancyReferencingAssemblies()
         private static IEnumerable<Assembly> LoadNancyReferencingAssemblies(IEnumerable<Assembly> loadedAssemblies)
         {
             var assemblies = new HashSet<Assembly>();
-            var inspectionAppDomain = AppDomain.CreateDomain("AppDomainAssemblyCatalog");
+            var inspectionAppDomain = CreateInspectionAppDomain();
+            var inspectionProber = CreateRemoteReferenceProber(inspectionAppDomain);
             var loadedNancyReferencingAssemblyNames = loadedAssemblies.Select(assembly => assembly.GetName()).ToArray();
 
             foreach (var directory in GetAssemblyDirectories())
@@ -68,9 +71,7 @@ private static IEnumerable<Assembly> LoadNancyReferencingAssemblies(IEnumerable<
 
                     if (!loadedNancyReferencingAssemblyNames.Any(loadedNancyReferencingAssemblyName => AssemblyName.ReferenceMatchesDefinition(loadedNancyReferencingAssemblyName, unloadedAssemblyName)))
                     {
-                        var inspectionAssembly = SafeLoadAssembly(inspectionAppDomain, unloadedAssemblyName);
-
-                        if (inspectionAssembly != null && IsNancyReferencing(inspectionAssembly))
+                        if (inspectionProber.HasReference(unloadedAssemblyName, NancyAssemblyName))
                         {
                             var assembly = SafeLoadAssembly(AppDomain.CurrentDomain, unloadedAssemblyName);
 
@@ -88,22 +89,19 @@ private static IEnumerable<Assembly> LoadNancyReferencingAssemblies(IEnumerable<
             return assemblies.ToArray();
         }
 
-        private static bool IsNancyReferencing(Assembly assembly)
+        private static AppDomain CreateInspectionAppDomain()
         {
-            if (AssemblyName.ReferenceMatchesDefinition(assembly.GetName(), NancyAssemblyName))
-            {
-                return true;
-            }
+            var currentAppDomain = AppDomain.CurrentDomain;
 
-            foreach (var referencedAssemblyName in assembly.GetReferencedAssemblies())
-            {
-                if (AssemblyName.ReferenceMatchesDefinition(referencedAssemblyName, NancyAssemblyName))
-                {
-                    return true;
-                }
-            }
+            return AppDomain.CreateDomain("AppDomainAssemblyCatalog", currentAppDomain.Evidence,
+                currentAppDomain.SetupInformation);
+        }
 
-            return false;
+        private static ProxyNancyReferenceProber CreateRemoteReferenceProber(AppDomain appDomain)
+        {
+            return (ProxyNancyReferenceProber)appDomain.CreateInstanceAndUnwrap(
+                typeof(ProxyNancyReferenceProber).Assembly.FullName,
+                typeof(ProxyNancyReferenceProber).FullName);
         }
 
         private static IEnumerable<string> GetAssemblyDirectories()

src/Nancy/Extensions/AssemblyExtensions.cs

@@ -37,5 +37,31 @@ public static Type[] SafeGetExportedTypes(this Assembly assembly)
             }
             return types;
         }
+
+#if !CORE
+        /// <summary>
+        /// Indicates if a given assembly references another which is identified by its name.
+        /// </summary>
+        /// <param name="assembly">The assembly which will be probed.</param>
+        /// <param name="referenceName">The reference assembly name.</param>
+        /// <returns>A boolean value indicating if there is a reference.</returns>
+        public static bool IsReferencing(this Assembly assembly, AssemblyName referenceName)
+        {
+            if (AssemblyName.ReferenceMatchesDefinition(assembly.GetName(), referenceName))
+            {
+                return true;
+            }
+
+            foreach (var referencedAssemblyName in assembly.GetReferencedAssemblies())
+            {
+                if (AssemblyName.ReferenceMatchesDefinition(referencedAssemblyName, referenceName))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+#endif
     }
 }

src/Nancy/Helpers/ProxyNancyReferenceProber.cs

@@ -0,0 +1,30 @@
+#if !CORE
+namespace Nancy.Helpers
+{
+    using System;
+    using System.Reflection;
+    using Nancy.Extensions;
+
+    /// <summary>
+    /// Utility class used to probe assembly references.
+    /// </summary>
+    /// <remarks>
+    /// Because this class inherits from <see cref="T:System.MarshalByRefObject"/> it can be used across different <see cref="T:System.AppDomain"/>.
+    /// </remarks>
+    internal class ProxyNancyReferenceProber : MarshalByRefObject
+    {
+        /// <summary>
+        /// Determines if the assembly has a reference (dependency) upon another one.
+        /// </summary>
+        /// <param name="assemblyNameForProbing">The name of the assembly that will be tested.</param>
+        /// <param name="referenceAssemblyName">The reference assembly name.</param>
+        /// <returns>A boolean value indicating if there is a reference.</returns>
+        public bool HasReference(AssemblyName assemblyNameForProbing, AssemblyName referenceAssemblyName)
+        {
+            var assemblyForInspection = Assembly.ReflectionOnlyLoad(assemblyNameForProbing.Name);
+
+            return assemblyForInspection.IsReferencing(referenceAssemblyName);
+        }
+    }
+}
+#endif

test/Nancy.Tests.MSBuild/Nancy.Tests.csproj

@@ -649,6 +649,9 @@
     <Compile Include="..\Nancy.Tests\Unit\Responses\GenericFileResponseFixture.cs">
       <Link>Unit\Responses\GenericFileResponseFixture.cs</Link>
     </Compile>
+    <Compile Include="..\Nancy.Tests\Unit\AppDomainAssemblyCatalogFixture.cs">
+      <Link>Unit\AppDomainAssemblyCatalogFixture.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\src\Nancy.Testing.MSBuild\Nancy.Testing.csproj">
@@ -744,4 +747,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>

test/Nancy.Tests/Unit/AppDomainAssemblyCatalogFixture.cs

@@ -0,0 +1,71 @@
+namespace Nancy.Tests.Unit
+{
+    using System;
+    using System.CodeDom.Compiler;
+    using System.Linq;
+    using System.Reflection;
+    using Xunit;
+
+    public class AppDomainAssemblyCatalogFixture
+    {
+        [Fact]
+        public void Modules_without_Nancy_references_should_not_keep_loaded_after_inspection()
+        {
+            // Given
+            var compilerAppDomain = AppDomain.CreateDomain("AssemblyGenerator",
+                AppDomain.CurrentDomain.Evidence, AppDomain.CurrentDomain.SetupInformation);
+
+            var assemblyGenerator = (ProxyAssemblyGenerator)compilerAppDomain.CreateInstanceAndUnwrap(
+                typeof(ProxyAssemblyGenerator).Assembly.FullName,
+                typeof(ProxyAssemblyGenerator).FullName);
+
+            try
+            {
+                var generatedAssemblyName = assemblyGenerator.GenerateAssemblyAndGetName();
+
+                var assemblyCatalog = new AppDomainAssemblyCatalog();
+
+                // When
+
+                // the following call will load the assemblies into its own inspection AppDomain
+                // and release the assemblies that do not reference Nancy afterwards, 
+                // keeping the application AppDomain free of such assemblies, that is, the created
+                // assembly should not be loaded by AppDomain.GetAssemblies after this call
+                assemblyCatalog.GetAssemblies();
+
+                var loadedAssembliesAfterInspection = AppDomain.CurrentDomain.GetAssemblies();
+
+                // Then
+                loadedAssembliesAfterInspection
+                    .Select(assembly => assembly.GetName().Name)
+                    .Contains(generatedAssemblyName.Name)
+                    .ShouldBeFalse();
+            }
+            finally
+            {
+                AppDomain.Unload(compilerAppDomain);
+            }
+        }
+
+        private class ProxyAssemblyGenerator : MarshalByRefObject
+        {
+            public AssemblyName GenerateAssemblyAndGetName()
+            {
+                var generatedAssembly = CodeDomProvider
+                    .CreateProvider("CSharp")
+                    .CompileAssemblyFromSource(
+                        new CompilerParameters
+                        {
+                            GenerateInMemory = true,
+                            GenerateExecutable = false,
+                            IncludeDebugInformation = false,
+                            OutputAssembly = "AssemblyShouldNotBeLoadedIntoAppDomain.dll"
+                        },
+                        "public class DummyClass { }")
+                    .CompiledAssembly;
+
+                return generatedAssembly.GetName();
+            }
+        }
+    }
+}