Repository navigation
Make plugin and module loading fault-tolerant, safe and isolated - #1020
Merged
Merged
Conversation
added 25 commits
November 2, 2026 08:45
…p incompatible plugins out of dependency dedupe
…ugin when rollback fails
…raries A library may hold types built on an optional assembly it never ships; GetTypes on it failed the whole plugin although it works at runtime. The check now covers the main assembly and the assemblies the reference check already treats as plugin code.
The target is the existing folder matched case-insensitively, under its exact name, so a case-variant archive root replaces it in place on every platform. An existing folder holding a plugin is only replaced by an archive with the same system name. The comment on a kept temp folder now says it stays until an operator removes it.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| context.Unload(); | ||
| for (var i = 0; i < 10; i++) | ||
| { | ||
| GC.Collect(); |
Comment on lines
+199
to
+209
| foreach (var file in Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories)) | ||
| { | ||
| var relative = Path.GetRelativePath(root, file); | ||
| var separator = relative.IndexOf(Path.DirectorySeparatorChar); | ||
| if (separator < 0) | ||
| throw new GrandException("The archive should not contain files outside of the plugin folder."); | ||
|
|
||
| var name = relative[..separator]; | ||
| if (!names.Contains(name, StringComparer.OrdinalIgnoreCase)) | ||
| names.Add(name); | ||
| } |
Comment on lines
+45
to
+65
| foreach (var handle in reader.GetAssemblyDefinition().GetCustomAttributes()) | ||
| { | ||
| var attribute = reader.GetCustomAttribute(handle); | ||
| if (!IsPluginInfoAttribute(reader, attribute.Constructor)) | ||
| continue; | ||
|
|
||
| var named = new Dictionary<string, string>(StringComparer.Ordinal); | ||
| foreach (var argument in attribute.DecodeValue(provider).NamedArguments) | ||
| if (!named.TryAdd(argument.Name!, argument.Value as string)) | ||
| throw new BadImageFormatException( | ||
| $"The PluginInfo attribute of '{Path.GetFileName(assemblyPath)}' sets {argument.Name} more than once."); | ||
|
|
||
| return new PluginManifest( | ||
| named.GetValueOrDefault(nameof(PluginInfoAttribute.SystemName)), | ||
| named.GetValueOrDefault(nameof(PluginInfoAttribute.FriendlyName)), | ||
| named.GetValueOrDefault(nameof(PluginInfoAttribute.Group)), | ||
| named.GetValueOrDefault(nameof(PluginInfoAttribute.Author)), | ||
| named.GetValueOrDefault(nameof(PluginInfoAttribute.Version)), | ||
| PluginVersionResolver.ResolveSupportedVersion(GetCoreReference(reader), | ||
| named.GetValueOrDefault(nameof(PluginInfoAttribute.SupportedVersion)))); | ||
| } |
Comment on lines
+87
to
+92
| foreach (var handle in reader.AssemblyReferences) | ||
| { | ||
| var reference = reader.GetAssemblyReference(handle); | ||
| if (string.Equals(reader.GetString(reference.Name), CoreAssemblyName, StringComparison.OrdinalIgnoreCase)) | ||
| return reference.Version; | ||
| } |
Comment on lines
+365
to
+372
| catch (Exception rollback) | ||
| { | ||
| logger.LogError(rollback, | ||
| "The plugin could not be installed and the previous version could not be restored; it is kept in {PreviousPath}", | ||
| previous); | ||
| throw new RollbackFailedException( | ||
| $"The plugin could not be installed and the previous version could not be restored. The previous version is preserved at '{previous}'."); | ||
| } |
Comment on lines
+82
to
+87
| catch (Exception ex) | ||
| { | ||
| //FileLoadException, FileNotFoundException, BadImageFormatException, an unresolvable deps.json and | ||
| //whatever else a plugin's files can cause; the plugin is reported, the application goes on | ||
| return Failed(plugin, ex.Message); | ||
| } |
added 2 commits
November 4, 2026 21:20
…s not identified An entry for a folder that could not be loaded has no main assembly, so Remove failed on it, and two folders declaring one system name were told apart by neither: Remove deleted the first, the working one. The list now carries the folder name, and Remove finds the entry by system name and folder.
The installed plugin of that name is the folder that loaded; the second one was listed installed, so FailOnPluginLoadError stopped the application although the plugin was working, and the list asked to uninstall it before removal.
added 4 commits
November 4, 2026 21:21
Only the top-level files of Plugins/bin were cleared at startup, so each removed or renamed plugin folder left its whole copy there for good.
Assemblies were de-duplicated by full name, but every plugin now loads its files into a context of its own: only the first plugin's copy of a shared library was scanned, so the second plugin's types from it were never registered. One assembly is now kept per load context.
…lder, as pending A loaded plugin was compared by its main assembly only, and a folder that existed at load time was never read again, so neither change showed the restart notice nor stopped Install and Uninstall before the restart. All top-level dlls are compared now, and a folder that had no plugin is read again once its dlls change.
Comment on lines
+118
to
+129
| foreach (var folder in folders.Where(x => !_plugins.ContainsKey(x)).Order(StringComparer.Ordinal)) | ||
| { | ||
| if (_otherFolders.TryGetValue(folder, out var stamp) && | ||
| stamp == StampOfAssemblies(Path.Combine(_pluginsFolder, folder))) | ||
| continue; | ||
|
|
||
| var manifest = ReadManifest(Path.Combine(_pluginsFolder, folder)); | ||
| if (manifest != null) | ||
| changes.Add(new PluginPendingChange( | ||
| string.IsNullOrWhiteSpace(manifest.SystemName) ? folder : manifest.SystemName, folder, | ||
| PluginPendingChangeKind.Added)); | ||
| } |
Comment on lines
+112
to
+126
| foreach (var copy in _copyFolder.GetDirectories()) | ||
| { | ||
| if (plugin.SupportedVersion != GrandVersion.SupportedPluginVersion) | ||
| { | ||
| _logger.LogInformation("Incompatible plugin {PluginSystemName}", plugin.SystemName); | ||
| //set as not installed | ||
| referencedPlugins.Add(plugin); | ||
| if (Directory.Exists(Path.Combine(_pluginFolder.FullName, copy.Name))) | ||
| continue; | ||
| } | ||
|
|
||
| //some validation | ||
| if (string.IsNullOrWhiteSpace(plugin.SystemName)) | ||
| throw new Exception($"The plugin '{plugin.SystemName}' has no system name."); | ||
| if (referencedPlugins.Contains(plugin)) | ||
| throw new Exception($"The plugin with '{plugin.SystemName}' system name is already defined"); | ||
|
|
||
| //set 'Installed' property | ||
| plugin.Installed = installedPluginSystemNames | ||
| .FirstOrDefault(x => x.Equals(plugin.SystemName, StringComparison.OrdinalIgnoreCase)) != null; | ||
|
|
||
| _logger.LogInformation("Deleting the shadow copy of the removed plugin folder {FolderName}", copy.Name); | ||
| try | ||
| { | ||
| if (!_config.PluginShadowCopy) | ||
| { | ||
| //remove deps.json files | ||
| var depsFiles = | ||
| plugin.OriginalAssemblyFile.Directory!.GetFiles("*.deps.json", | ||
| SearchOption.TopDirectoryOnly); | ||
| foreach (var f in depsFiles) | ||
| try | ||
| { | ||
| File.Delete(f.FullName); | ||
| } | ||
| catch (Exception exc) | ||
| { | ||
| _logger.LogError(exc, "PluginManager"); | ||
| } | ||
| } | ||
|
|
||
| //controllers and views only for an installed plugin - an uninstalled one must not be routable | ||
| if (plugin.Installed) | ||
| AddApplicationPart(mvcCoreBuilder, plugin.ReferencedAssembly, plugin.SystemName, | ||
| plugin.PluginFileName); | ||
|
|
||
| //register interface for IPlugin - also when not installed, Install resolves it from DI | ||
| RegisterPluginInterface(mvcCoreBuilder, plugin.ReferencedAssembly); | ||
|
|
||
| //init plugin type | ||
| foreach (var t in plugin.ReferencedAssembly.GetTypes()) | ||
| if (typeof(IPlugin).IsAssignableFrom(t)) | ||
| if (!t.GetTypeInfo().IsInterface) | ||
| if (t.GetTypeInfo().IsClass && !t.GetTypeInfo().IsAbstract) | ||
| { | ||
| plugin.PluginType = t; | ||
| break; | ||
| } | ||
|
|
||
| referencedPlugins.Add(plugin); | ||
| copy.Delete(true); | ||
| } | ||
| catch (ReflectionTypeLoadException ex) | ||
| catch (Exception exc) when (exc is IOException or UnauthorizedAccessException) | ||
| { | ||
| var msg = $"Plugin '{plugin.FriendlyName}'. "; | ||
| msg = ex.LoaderExceptions.Aggregate(msg, | ||
| (current, e) => current + e!.Message + Environment.NewLine); | ||
|
|
||
| var fail = new Exception(msg, ex); | ||
| throw fail; | ||
| _logger.LogWarning(exc, "Shadow copy {FullName} could not be deleted", copy.FullName); | ||
| } | ||
| catch (Exception ex) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type: bugfix
Issue
How plugins and modules are loaded and handled had several defects, all reproduced on a Release publish of
develop(5e8cf26):Extensions:PluginShadowCopy=false(what the Aspire AppHost sets) the store did not start with the bundled plugins: Payments.BrainTree ships Newtonsoft.Json 13.0.1 and Payments.StripeCheckout ships 13.0.3, and both were loaded into the default context ("Assembly with same name is already loaded"). With shadow copy on, the first copy won silently.IPlugin, or a non-.NET*.dll.PluginInfoAttribute.SupportedVersion, which is null by convention. Archives without directory entries were rejected too. An archive named after an existing plugin could delete that plugin.IStartupApplicationran.SystemNamecheck never fired.Extensions:InstalledPluginswas set.InstalledPlugins.cfgwas written non-atomically and removed names case-sensitively. Plugin activation errors lost their cause.ModuleLoaderadded every module dll as an MVC part and loadedGrand.Module.Scim's own copies of host assemblies.Solution
PluginManifestReader) and the load is planned before anything is loaded (PluginFolderScanner).PluginLoader). A faulty plugin gets aLoadErrorthat is logged with its folder and reason and shown on the admin plugin list. The store still starts. The newExtensions:FailOnPluginLoadErrorsetting (defaultfalse) makes it fail fast instead.AssemblyLoadContext(IsolatedLoadContext). It resolves dependencies from the plugin'sdeps.jsonand takes host assemblies from the default context. Plugins with different versions of a library run side by side. Shadow copy now works per folder (Plugins/bin/<Folder>). Modules use the same model.PluginArchiveInstaller):InstalledPlugins.cfgis written atomically under a lock. Invalid JSON fails with the file path. Activation errors keep their cause.TypeSearcher). In the test host, warm scans dropped from 3–8 ms to under 0.3 ms; this was not measured on a published store.Breaking changes
For third-party plugins:
LoadError. Before, it worked by accident, because all plugins shared one context. The same applies to a module that references another module.deps.jsonis now used to resolve its dependencies, so it must be consistent: a listed file that is missing fails the plugin.PluginInfo.ReferencedAssemblyandPluginTypeare null for a plugin that is not installed.Instance()loads it on demand and creates it withActivatorUtilities, so anIPluginconstructor may depend only on host services.IPluginclass is registered in DI.ITypeSearcher.GetAssemblies()returns a shared, read-only list.For admins:
Extensions:FailOnPluginLoadError, defaultfalse. With the default, existing behaviour is kept except that a faulty plugin no longer stops the store.en_250.xml. Existing keys keep their values.Testing
dotnet test GrandNode.slnx: all green.Grand.WebRelease with all plugins and modules. RemoveApp_Data/Settings.cfgfrom the publish. SetDatabase__UseLiteDb=trueand install a store with sample data.Extensions__PluginShadowCopy=false. In each run, check:native.dll, intoPlugins. The store starts and the admin list shows the errors. WithExtensions__FailOnPluginLoadError=trueand the broken plugin installed, startup stops with the plugin named.Compress-Archive): it installs. An archive whose root folder is an existing plugin's folder but holds another plugin is rejected.Grand.Module.ApiandGrand.Module.Scim:/scim/v2/Usersreturns 401 without a token and 200 with one;Not verified: real BrainTree and Stripe payments, Google and Facebook sign-in, MongoDB, the Theme.Modern render, and a two-replica Aspire start.