refactor(core): harden PluginLoader per code review
Addresses code quality review of a7f0732:
- LoadedPlugin now holds the AssemblyLoadContext explicitly so Task 10
can call Unload() for hot reload (Critical)
- LoadedPlugin.Error is Exception? to match PluginDiscoveryResult and
preserve stack traces; synthetic failures build FileNotFoundException
and InvalidOperationException (Important)
- PluginLoader falls back to ReflectionTypeLoadException.Types if
GetTypes() can't fully resolve (Important)
- Hardcoded abstractions assembly name is now a const (Minor)
This commit is contained in:
parent
a7f0732026
commit
f6a57cbc6c
4 changed files with 45 additions and 8 deletions
|
|
@ -1,11 +1,20 @@
|
||||||
|
using System.Runtime.Loader;
|
||||||
using AcDream.Plugin.Abstractions;
|
using AcDream.Plugin.Abstractions;
|
||||||
|
|
||||||
namespace AcDream.Core.Plugins;
|
namespace AcDream.Core.Plugins;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Outcome of a plugin load attempt.
|
||||||
|
/// <para>On success, <see cref="Plugin"/> is the instantiated plugin, <see cref="LoadContext"/>
|
||||||
|
/// owns its assembly, and <see cref="Error"/> is null.</para>
|
||||||
|
/// <para>On failure, <see cref="Plugin"/> and <see cref="LoadContext"/> are null and
|
||||||
|
/// <see cref="Error"/> describes what went wrong.</para>
|
||||||
|
/// </summary>
|
||||||
public sealed record LoadedPlugin(
|
public sealed record LoadedPlugin(
|
||||||
PluginManifest Manifest,
|
PluginManifest Manifest,
|
||||||
IAcDreamPlugin? Plugin,
|
IAcDreamPlugin? Plugin,
|
||||||
string? Error)
|
AssemblyLoadContext? LoadContext,
|
||||||
|
Exception? Error)
|
||||||
{
|
{
|
||||||
public bool Success => Plugin is not null && Error is null;
|
public bool Success => Plugin is not null && Error is null;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,8 @@ namespace AcDream.Core.Plugins;
|
||||||
/// </summary>
|
/// </summary>
|
||||||
internal sealed class PluginAssemblyLoadContext : AssemblyLoadContext
|
internal sealed class PluginAssemblyLoadContext : AssemblyLoadContext
|
||||||
{
|
{
|
||||||
|
private const string AbstractionsAssemblyName = "AcDream.Plugin.Abstractions";
|
||||||
|
|
||||||
private readonly AssemblyDependencyResolver _resolver;
|
private readonly AssemblyDependencyResolver _resolver;
|
||||||
|
|
||||||
public PluginAssemblyLoadContext(string pluginDirectory, string pluginEntryPath)
|
public PluginAssemblyLoadContext(string pluginDirectory, string pluginEntryPath)
|
||||||
|
|
@ -21,7 +23,7 @@ internal sealed class PluginAssemblyLoadContext : AssemblyLoadContext
|
||||||
protected override Assembly? Load(AssemblyName assemblyName)
|
protected override Assembly? Load(AssemblyName assemblyName)
|
||||||
{
|
{
|
||||||
// Share the abstractions assembly with the host — do NOT reload it in the plugin ALC
|
// Share the abstractions assembly with the host — do NOT reload it in the plugin ALC
|
||||||
if (assemblyName.Name == "AcDream.Plugin.Abstractions")
|
if (assemblyName.Name == AbstractionsAssemblyName)
|
||||||
return null;
|
return null;
|
||||||
|
|
||||||
var path = _resolver.ResolveAssemblyToPath(assemblyName);
|
var path = _resolver.ResolveAssemblyToPath(assemblyName);
|
||||||
|
|
|
||||||
|
|
@ -5,30 +5,56 @@ namespace AcDream.Core.Plugins;
|
||||||
|
|
||||||
public static class PluginLoader
|
public static class PluginLoader
|
||||||
{
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// Load a plugin DLL from <paramref name="pluginDirectory"/> into a collectible
|
||||||
|
/// <see cref="System.Runtime.Loader.AssemblyLoadContext"/>, find the first type
|
||||||
|
/// implementing <see cref="IAcDreamPlugin"/>, instantiate it, and call its
|
||||||
|
/// <see cref="IAcDreamPlugin.Initialize"/> with the supplied host. Any failure
|
||||||
|
/// is returned as a failed <see cref="LoadedPlugin"/> rather than thrown.
|
||||||
|
/// </summary>
|
||||||
public static LoadedPlugin Load(string pluginDirectory, PluginManifest manifest, IPluginHost host)
|
public static LoadedPlugin Load(string pluginDirectory, PluginManifest manifest, IPluginHost host)
|
||||||
{
|
{
|
||||||
var dllPath = Path.Combine(pluginDirectory, manifest.EntryDll);
|
var dllPath = Path.Combine(pluginDirectory, manifest.EntryDll);
|
||||||
if (!File.Exists(dllPath))
|
if (!File.Exists(dllPath))
|
||||||
return new LoadedPlugin(manifest, null, $"entry dll not found: {dllPath}");
|
return new LoadedPlugin(
|
||||||
|
manifest,
|
||||||
|
Plugin: null,
|
||||||
|
LoadContext: null,
|
||||||
|
Error: new FileNotFoundException($"entry dll not found: {dllPath}", dllPath));
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var alc = new PluginAssemblyLoadContext(pluginDirectory, dllPath);
|
var alc = new PluginAssemblyLoadContext(pluginDirectory, dllPath);
|
||||||
var asm = alc.LoadFromAssemblyPath(dllPath);
|
var asm = alc.LoadFromAssemblyPath(dllPath);
|
||||||
|
|
||||||
var pluginType = asm.GetTypes()
|
IEnumerable<Type> types;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
types = asm.GetTypes();
|
||||||
|
}
|
||||||
|
catch (ReflectionTypeLoadException rtle)
|
||||||
|
{
|
||||||
|
types = rtle.Types.OfType<Type>();
|
||||||
|
}
|
||||||
|
|
||||||
|
var pluginType = types
|
||||||
.FirstOrDefault(t => !t.IsAbstract && typeof(IAcDreamPlugin).IsAssignableFrom(t));
|
.FirstOrDefault(t => !t.IsAbstract && typeof(IAcDreamPlugin).IsAssignableFrom(t));
|
||||||
|
|
||||||
if (pluginType is null)
|
if (pluginType is null)
|
||||||
return new LoadedPlugin(manifest, null, $"no IAcDreamPlugin implementation found in {manifest.EntryDll}");
|
return new LoadedPlugin(
|
||||||
|
manifest,
|
||||||
|
Plugin: null,
|
||||||
|
LoadContext: null,
|
||||||
|
Error: new InvalidOperationException(
|
||||||
|
$"no IAcDreamPlugin implementation found in {manifest.EntryDll}"));
|
||||||
|
|
||||||
var instance = (IAcDreamPlugin)Activator.CreateInstance(pluginType)!;
|
var instance = (IAcDreamPlugin)Activator.CreateInstance(pluginType)!;
|
||||||
instance.Initialize(host);
|
instance.Initialize(host);
|
||||||
return new LoadedPlugin(manifest, instance, null);
|
return new LoadedPlugin(manifest, instance, alc, Error: null);
|
||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
return new LoadedPlugin(manifest, null, $"{ex.GetType().Name}: {ex.Message}");
|
return new LoadedPlugin(manifest, Plugin: null, LoadContext: null, Error: ex);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -97,6 +97,6 @@ public class PluginLoaderTests
|
||||||
var loaded = PluginLoader.Load(coreDllDir, manifest, host);
|
var loaded = PluginLoader.Load(coreDllDir, manifest, host);
|
||||||
|
|
||||||
Assert.False(loaded.Success);
|
Assert.False(loaded.Success);
|
||||||
Assert.Contains("IAcDreamPlugin", loaded.Error ?? "");
|
Assert.Contains("IAcDreamPlugin", loaded.Error!.Message);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue