Make plugin and module loading fault-tolerant, safe and isolated

src/Build/Grand.Plugin.props

@@ -18,8 +18,8 @@
     <!--
         The host assemblies every plugin compiles against.
 
-        Private=false keeps them out of the plugin's output folder: the plugin is loaded
-        into the host's AssemblyLoadContext, which already has them, and a second copy of
+        Private=false keeps them out of the plugin's output folder: the plugin's own load
+        context takes them from the host's, which already has them, and a second copy of
         a host assembly beside a plugin is a hazard - if that copy is ever the one loaded,
         its types are not the host's types and DI resolution fails in ways that are hard
         to trace.

src/Core/Grand.Infrastructure/Configuration/ExtensionsConfig.cs

@@ -11,7 +11,8 @@ public class ExtensionsConfig
     public bool DisableUploadExtensions { get; set; }
 
     /// <summary>
-    ///     Gets or sets a value indicating whether copy dll plugin files to /Plugins/bin on application startup
+    ///     Gets or sets a value indicating whether each installed plugin's folder is copied to /Plugins/bin/{folder}
+    ///     on application startup and loaded from there
     /// </summary>
     public bool PluginShadowCopy { get; set; }
 
@@ -29,4 +30,12 @@ public class ExtensionsConfig
     ///     List of installed plugins (comma separated)
     /// </summary>
     public string InstalledPlugins { get; set; }
+
+    /// <summary>
+    ///     Stop the application when an installed plugin, or a module enabled under FeatureManagement, fails to
+    ///     load. Off: the plugin is logged, listed with its error and left inactive, the module is logged as
+    ///     critical, and the application starts without it. A module whose folder is not deployed with this
+    ///     application is not a load failure: it is logged as a warning either way.
+    /// </summary>
+    public bool FailOnPluginLoadError { get; set; }
 }

src/Core/Grand.Infrastructure/Grand.Infrastructure.csproj

@@ -22,4 +22,9 @@
     <ProjectReference Include="..\Grand.Mapping\Grand.Mapping.csproj" />
     <ProjectReference Include="..\Grand.Mediator\Grand.Mediator.csproj" />
   </ItemGroup>
+  <ItemGroup>
+    <InternalsVisibleTo Include="Grand.Infrastructure.Tests" />
+    <InternalsVisibleTo Include="Grand.Web.Admin.Tests" />
+    <InternalsVisibleTo Include="Grand.Web.Common.Tests" />
+  </ItemGroup>
 </Project>

src/Core/Grand.Infrastructure/Modules/ModuleLoader.cs

@@ -1,4 +1,7 @@
-using Microsoft.AspNetCore.Hosting;
+using Grand.Infrastructure.Configuration;
+using Grand.Infrastructure.Plugins;
+using Grand.SharedKernel;
+using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Mvc.ApplicationParts;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
@@ -9,12 +12,19 @@
 
 namespace Grand.Infrastructure.Modules;
 
+/// <summary>
+///     Loads the modules enabled under FeatureManagement, each into a load context of its own, as plugins are
+///     loaded: the module shares the host's assemblies and resolves its private libraries from its folder.
+/// </summary>
 public static class ModuleLoader
 {
     private const string MODULE_SECTION_PATH = "FeatureManagement";
     private const string MODULES_DIRECTORY = "Modules";
     private const string LOGGER_NAME = "ModuleManager";
 
+    //the contexts of modules that failed to load; their assemblies stay loaded, so scans must skip them
+    private static readonly ConditionalWeakTable<AssemblyLoadContext, object> FailedContexts = new();
+
     private static List<(string FeatureName, bool IsEnabled)> GetFeatureList(IConfiguration configuration)
     {
         var featureSection = configuration.GetSection(MODULE_SECTION_PATH);
@@ -30,29 +40,73 @@ public static class ModuleLoader
             ))
             .ToList();
     }
-
+
+    /// <summary>
+    ///     Loads the enabled modules. A module that cannot be loaded is logged and left out, or, with
+    ///     Extensions:FailOnPluginLoadError, stops the application.
+    /// </summary>
+    /// <exception cref="GrandException">A module failed to load and Extensions:FailOnPluginLoadError is set</exception>
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static void LoadModules(IMvcCoreBuilder mvcCoreBuilder, IConfiguration configuration, IWebHostEnvironment hostingEnvironment)
+    {
+        LoadModules(mvcCoreBuilder, configuration, hostingEnvironment,
+            (name, mainAssemblyPath, hostAssemblyNames) =>
+                new IsolatedLoadContext(name, mainAssemblyPath, hostAssemblyNames),
+            () => GetLoggerFactory(mvcCoreBuilder.Services));
+    }
+
+    /// <param name="createContext">
+    ///     Creates a module's own context from its name, main assembly path and the host assembly names; tests
+    ///     create collectible ones
+    /// </param>
+    /// <param name="getLoggerFactory">The logger factory, asked for only when there are modules to load</param>
+    internal static void LoadModules(IMvcCoreBuilder mvcCoreBuilder, IConfiguration configuration,
+        IWebHostEnvironment hostingEnvironment, Func<string, string, ISet<string>, AssemblyLoadContext> createContext,
+        Func<ILoggerFactory> getLoggerFactory)
     {
         var modules = GetFeatureList(configuration);
         if (modules.Count == 0)
         {
             return;
         }
-        var logger = CreateLogger(mvcCoreBuilder.Services);
+
+        var config = new ExtensionsConfig();
+        configuration.GetSection("Extensions").Bind(config);
+        var logger = getLoggerFactory().CreateLogger(LOGGER_NAME);
+        var hostAssemblyNames = PluginManager.HostAssemblyNames();
         foreach (var module in modules)
         {
             if (!module.IsEnabled)
             {
                 logger.LogInformation("Module '{ModuleName}' is disabled.", module.FeatureName);
                 continue;
             }
-            LoadModule(module.FeatureName, hostingEnvironment, mvcCoreBuilder, logger);
+
+            var error = LoadModule(module.FeatureName, hostingEnvironment, mvcCoreBuilder, logger,
+                mainAssemblyPath => createContext(module.FeatureName, mainAssemblyPath, hostAssemblyNames));
+            if (error == null)
+                continue;
+
+            logger.LogCritical("Module '{ModuleName}' could not be loaded: {LoadError}", module.FeatureName, error);
+            if (config.FailOnPluginLoadError)
+                throw new GrandException($"The module '{module.FeatureName}' could not be loaded: {error}");
         }
     }
 
-    private static void LoadModule(string moduleName, IWebHostEnvironment hostingEnvironment,
-            IMvcCoreBuilder mvcCoreBuilder, ILogger logger)
+    /// <summary>
+    ///     Whether the assembly belongs to a module that failed to load; its types must not be scanned
+    /// </summary>
+    internal static bool IsFailedModule(Assembly assembly)
+    {
+        ArgumentNullException.ThrowIfNull(assembly);
+
+        var context = AssemblyLoadContext.GetLoadContext(assembly);
+        return context != null && FailedContexts.TryGetValue(context, out _);
+    }
+
+    /// <returns>The reason the module could not be loaded; null when it is loaded, or has no files to load</returns>
+    private static string LoadModule(string moduleName, IWebHostEnvironment hostingEnvironment,
+        IMvcCoreBuilder mvcCoreBuilder, ILogger logger, Func<string, AssemblyLoadContext> createContext)
     {
         var modulePath = GetModulePath(hostingEnvironment, moduleName);
         var moduleFile = Path.Combine(modulePath, $"{moduleName}.dll");
@@ -61,73 +115,81 @@ private static void LoadModule(string moduleName, IWebHostEnvironment hostingEnv
         {
             logger.LogWarning("Module '{ModuleName}' does not exist at path: {ModulePath}",
                 moduleName, moduleFile);
-            return;
+            return null;
         }
 
+        AssemblyLoadContext context = null;
         try
         {
-            var assemblyLoadContext = new ModuleLoadContext();
-            LoadModuleDependencies(modulePath, assemblyLoadContext, mvcCoreBuilder, logger);
+            context = createContext(moduleFile);
+            var assembly = context.LoadFromAssemblyPath(moduleFile);
+
+            //resolved now, so a dependency the context cannot provide fails the module here instead of a type scan
+            //or a request
+            _ = assembly.GetTypes();
+            var error = ResolveReferences(assembly, context);
+            if (error != null)
+            {
+                FailedContexts.AddOrUpdate(context, null);
+                return error;
+            }
+
+            AddApplicationPart(mvcCoreBuilder, assembly);
             logger.LogInformation("Module '{ModuleName}' has been successfully loaded.", moduleName);
+            return null;
         }
         catch (Exception ex)
         {
-            logger.LogError(ex, "Failed to load module '{ModuleName}'. Error: {Error}",
-                moduleName, ex.Message);
+            if (context != null)
+                FailedContexts.AddOrUpdate(context, null);
+
+            return ex is ReflectionTypeLoadException typeLoad
+                ? typeLoad.LoaderExceptions.FirstOrDefault(x => x != null)?.Message ?? ex.Message
+                : ex.Message;
         }
     }
-    private static void LoadModuleDependencies(string modulePath, ModuleLoadContext assemblyLoadContext,
-            IMvcCoreBuilder mvcCoreBuilder, ILogger logger)
-    {
-        var loadedAssemblies = AppDomain.CurrentDomain.GetAssemblies()
-            .Select(a => a.Location)
-            .ToHashSet();
 
-        foreach (var dependencyDll in Directory.GetFiles(modulePath, "*.dll"))
+    /// <summary>
+    ///     Resolves every assembly the module references. The application's copy of an assembly wins over the
+    ///     module's, and the runtime binds it whatever its version, so one older than the module was built against
+    ///     is reported here: it would fail only once a request reached the member it lacks.
+    /// </summary>
+    /// <returns>The reason the module cannot use its references; null when it can</returns>
+    private static string ResolveReferences(Assembly assembly, AssemblyLoadContext context)
+    {
+        foreach (var reference in assembly.GetReferencedAssemblies())
         {
-            if (loadedAssemblies.Contains(dependencyDll))
-                continue;
-
-            try
-            {
-                var assembly = assemblyLoadContext.LoadFromAssemblyPath(dependencyDll);
-                AddApplicationPart(mvcCoreBuilder, assembly);
-                logger.LogDebug("Successfully loaded dependency: {Dependency}", dependencyDll);
-            }
-            catch (Exception ex)
-            {
-                logger.LogWarning(ex, "Failed to load dependency: {Dependency}", dependencyDll);
-            }
+            var resolved = context.LoadFromAssemblyName(reference).GetName();
+            if (reference.Version != null && resolved.Version != null && resolved.Version < reference.Version)
+                return $"it is built against {reference.Name} {reference.Version}, " +
+                       $"but the application provides {resolved.Version}";
         }
+
+        return null;
     }
 
+    /// <summary>
+    ///     The application's logging, with every provider configured so far (console, event log, OpenTelemetry)
+    /// </summary>
+    private static ILoggerFactory GetLoggerFactory(IServiceCollection services)
+        => services.BuildServiceProvider().GetService<ILoggerFactory>()
+           ?? throw new InvalidOperationException("Logger factory not found in services.");
+
     private static string GetModulePath(IWebHostEnvironment hostingEnvironment, string moduleName)
            => Path.Combine(hostingEnvironment.ContentRootPath, MODULES_DIRECTORY, moduleName);
 
-    private static ILogger CreateLogger(IServiceCollection services)
-        => services.BuildServiceProvider()
-            .GetService<ILoggerFactory>()
-            ?.CreateLogger(LOGGER_NAME)
-            ?? throw new InvalidOperationException("Logger factory not found in services.");
-
+    /// <summary>
+    ///     Adds the module's own assembly and its related assemblies; its dependencies are not application parts
+    /// </summary>
     private static void AddApplicationPart(IMvcCoreBuilder mvcCoreBuilder, Assembly assembly)
     {
-        mvcCoreBuilder.AddApplicationPart(assembly);
-
-        var relatedAssemblies = RelatedAssemblyAttribute.GetRelatedAssemblies(assembly, false);
-        foreach (var relatedAssembly in relatedAssemblies)
-        {
-            var applicationPartFactory = ApplicationPartFactory.GetApplicationPartFactory(relatedAssembly);
-            foreach (var part in applicationPartFactory.GetApplicationParts(relatedAssembly))
-                mvcCoreBuilder.PartManager.ApplicationParts.Add(part);
-        }
-    }
-
-    private class ModuleLoadContext : AssemblyLoadContext
-    {
-        public ModuleLoadContext() : base(isCollectible: true)
-        {
+        //related parts are read first, so a failure adds nothing
+        var relatedParts = RelatedAssemblyAttribute.GetRelatedAssemblies(assembly, false)
+            .SelectMany(x => ApplicationPartFactory.GetApplicationPartFactory(x).GetApplicationParts(x))
+            .ToList();
 
-        }
+        mvcCoreBuilder.AddApplicationPart(assembly);
+        foreach (var part in relatedParts)
+            mvcCoreBuilder.PartManager.ApplicationParts.Add(part);
     }
-}
+}

src/Core/Grand.Infrastructure/Plugins/IsolatedLoadContext.cs

@@ -0,0 +1,69 @@
+using System.Reflection;
+using System.Runtime.Loader;
+
+namespace Grand.Infrastructure.Plugins;
+
+/// <summary>
+///     The load context of one extension (a plugin or a module). It resolves the extension's own dependencies
+///     from its deps.json, or from its folder, and takes every assembly the application provides from the
+///     default context, so the extension shares the host's types while its private libraries, in whatever
+///     version it ships them, stay its own.
+/// </summary>
+/// <remarks>Not collectible: an extension stays loaded until the application stops.</remarks>
+public sealed class IsolatedLoadContext : AssemblyLoadContext
+{
+    private readonly AssemblyDependencyResolver _resolver;
+    private readonly string _folder;
+    private readonly ISet<string> _hostAssemblyNames;
+
+    /// <param name="name">The context's name, for diagnostics</param>
+    /// <param name="mainAssemblyPath">The extension's main assembly; its deps.json, if any, sits beside it</param>
+    /// <param name="hostAssemblyNames">Simple names of the assemblies the application provides</param>
+    /// <exception cref="InvalidOperationException">The extension's deps.json cannot be resolved</exception>
+    public IsolatedLoadContext(string name, string mainAssemblyPath, ISet<string> hostAssemblyNames)
+        : this(name, mainAssemblyPath, hostAssemblyNames, false)
+    {
+    }
+
+    /// <param name="isCollectible">Tests only: lets a test unload what it loaded</param>
+    internal IsolatedLoadContext(string name, string mainAssemblyPath, ISet<string> hostAssemblyNames,
+        bool isCollectible) : base(name, isCollectible)
+    {
+        ArgumentException.ThrowIfNullOrEmpty(mainAssemblyPath);
+        ArgumentNullException.ThrowIfNull(hostAssemblyNames);
+
+        _resolver = new AssemblyDependencyResolver(mainAssemblyPath);
+        _folder = Path.GetDirectoryName(Path.GetFullPath(mainAssemblyPath));
+        _hostAssemblyNames = hostAssemblyNames;
+    }
+
+    /// <summary>
+    ///     Null hands the name to the default context: an assembly the application provides, or one it has
+    ///     already loaded, is never loaded a second time.
+    /// </summary>
+    protected override Assembly Load(AssemblyName assemblyName)
+    {
+        var name = assemblyName.Name;
+        if (string.IsNullOrEmpty(name) || _hostAssemblyNames.Contains(name) || IsLoadedInDefault(name))
+            return null;
+
+        var path = _resolver.ResolveAssemblyToPath(assemblyName);
+        if (path != null)
+            return LoadFromAssemblyPath(path);
+
+        //a dll that sits in the folder without being listed in deps.json (or there is no deps.json)
+        var local = Path.Combine(_folder, name + ".dll");
+        return File.Exists(local) ? LoadFromAssemblyPath(local) : null;
+    }
+
+    protected override IntPtr LoadUnmanagedDll(string unmanagedDllName)
+    {
+        var path = _resolver.ResolveUnmanagedDllToPath(unmanagedDllName);
+        return path != null ? LoadUnmanagedDllFromPath(path) : IntPtr.Zero;
+    }
+
+    private static bool IsLoadedInDefault(string name)
+    {
+        return Default.Assemblies.Any(x => string.Equals(x.GetName().Name, name, StringComparison.OrdinalIgnoreCase));
+    }
+}