ToxicMushroom/Dalamud
1
0
Fork 0
mirror of https://github.com/goatcorp/Dalamud.git synced 2025-12-29 20:03:41 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Make ServiceScope IAsyncDisposable

Browse source 
ServiceScope.Dispose was not waiting for scoped services to complete
disposing. This had an effect of letting a new plugin instance register
a DtrBar entry before previous plugin instance's entry got unregistered.

This change also cleans up unloading procedure in LocalPlugin.
This commit is contained in:
Soreepeong 2024-08-18 07:58:45 +09:00
parent fdfdee1fcb
commit 0a8f9b73fb
7 changed files with 375 additions and 184 deletions
Download patch file Download diff file Expand all files Collapse all files

69
Dalamud/Plugin/Internal/PluginManager.cs
View file

@ -375,54 +375,39 @@ internal class PluginManager : IInternalDisposableService
/// <inheritdoc/>
void IInternalDisposableService.DisposeService()
{
var disposablePlugins =
this.installedPluginsList.Where(plugin => plugin.State is PluginState.Loaded or PluginState.LoadError).ToArray();
if (disposablePlugins.Any())
{
// Unload them first, just in case some of plugin codes are still running via callbacks initiated externally.
foreach (var plugin in disposablePlugins.Where(plugin => !plugin.Manifest.CanUnloadAsync))
{
try
{
plugin.UnloadAsync(true, false).Wait();
}
catch (Exception ex)
{
Log.Error(ex, $"Error unloading {plugin.Name}");
}
}
DisposeAsync(
this.installedPluginsList
.Where(plugin => plugin.State is PluginState.Loaded or PluginState.LoadError)
.ToArray(),
this.configuration).Wait();
return;
Task.WaitAll(disposablePlugins
.Where(plugin => plugin.Manifest.CanUnloadAsync)
.Select(plugin => Task.Run(async () =>
{
try
{
await plugin.UnloadAsync(true, false);
}
catch (Exception ex)
{
Log.Error(ex, $"Error unloading {plugin.Name}");
}
})).ToArray());
static async Task DisposeAsync(LocalPlugin[] disposablePlugins, DalamudConfiguration configuration)
{
if (disposablePlugins.Length == 0)
return;
// Any unload/dispose operation called from this function log errors on their own.
// Ignore all errors.
// Unload plugins that requires to be unloaded synchronously,
// just in case some plugin codes are still running via callbacks initiated externally.
foreach (var plugin in disposablePlugins.Where(plugin => !plugin.Manifest.CanUnloadAsync))
await plugin.UnloadAsync(PluginLoaderDisposalMode.None).SuppressException();
// Unload plugins that can be unloaded from any thread.
await Task.WhenAll(disposablePlugins.Select(plugin => plugin.UnloadAsync(PluginLoaderDisposalMode.None)))
.SuppressException();
// Just in case plugins still have tasks running that they didn't cancel when they should have,
// give them some time to complete it.
Thread.Sleep(this.configuration.PluginWaitBeforeFree ?? PluginWaitBeforeFreeDefault);
// This helps avoid plugins being reloaded from conflicting with itself of previous instance.
await Task.Delay(configuration.PluginWaitBeforeFree ?? PluginWaitBeforeFreeDefault);
// Now that we've waited enough, dispose the whole plugin.
// Since plugins should have been unloaded above, this should be done quickly.
foreach (var plugin in disposablePlugins)
{
try
{
plugin.Dispose();
}
catch (Exception e)
{
Log.Error(e, $"Error disposing {plugin.Name}");
}
}
// Since plugins should have been unloaded above, this should complete quickly.
await Task.WhenAll(disposablePlugins.Select(plugin => plugin.DisposeAsync().AsTask()))
.SuppressException();
}
// NET8 CHORE

7
Dalamud/Plugin/Internal/Types/LocalDevPlugin.cs
View file

@ -1,5 +1,4 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
@ -16,7 +15,7 @@ namespace Dalamud.Plugin.Internal.Types;
/// This class represents a dev plugin and all facets of its lifecycle.
/// The DLL on disk, dependencies, loaded assembly, etc.
/// </summary>
internal class LocalDevPlugin : LocalPlugin, IDisposable
internal class LocalDevPlugin : LocalPlugin
{
private static readonly ModuleLog Log = new("PLUGIN");
@ -101,7 +100,7 @@ internal class LocalDevPlugin : LocalPlugin, IDisposable
public List<string> DismissedValidationProblems => this.devSettings.DismissedValidationProblems;
/// <inheritdoc/>
public new void Dispose()
public override ValueTask DisposeAsync()
{
if (this.fileWatcher != null)
{
@ -110,7 +109,7 @@ internal class LocalDevPlugin : LocalPlugin, IDisposable
this.fileWatcher.Dispose();
}
base.Dispose();
return base.DisposeAsync();
}
/// <summary>

252
Dalamud/Plugin/Internal/Types/LocalPlugin.cs
View file

@ -1,6 +1,8 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.ExceptionServices;
using System.Threading;
using System.Threading.Tasks;
@ -20,7 +22,7 @@ namespace Dalamud.Plugin.Internal.Types;
/// This class represents a plugin and all facets of its lifecycle.
/// The DLL on disk, dependencies, loaded assembly, etc.
/// </summary>
internal class LocalPlugin : IDisposable
internal class LocalPlugin : IAsyncDisposable
{
/// <summary>
/// The underlying manifest for this plugin.
@ -41,6 +43,8 @@ internal class LocalPlugin : IDisposable
private Assembly? pluginAssembly;
private Type? pluginType;
private IDalamudPlugin? instance;
private IServiceScope? serviceScope;
private DalamudPluginInterface? dalamudInterface;
/// <summary>
/// Initializes a new instance of the <see cref="LocalPlugin"/> class.
@ -107,7 +111,7 @@ internal class LocalPlugin : IDisposable
/// <summary>
/// Gets the <see cref="DalamudPluginInterface"/> associated with this plugin.
/// </summary>
public DalamudPluginInterface? DalamudInterface { get; private set; }
public DalamudPluginInterface? DalamudInterface => this.dalamudInterface;
/// <summary>
/// Gets the path to the plugin DLL.
@ -220,40 +224,11 @@ internal class LocalPlugin : IDisposable
/// <summary>
/// Gets the service scope for this plugin.
/// </summary>
public IServiceScope? ServiceScope { get; private set; }
public IServiceScope? ServiceScope => this.serviceScope;
/// <inheritdoc/>
public void Dispose()
{
var framework = Service<Framework>.GetNullable();
var configuration = Service<DalamudConfiguration>.Get();
var didPluginDispose = false;
if (this.instance != null)
{
didPluginDispose = true;
if (this.manifest.CanUnloadAsync || framework == null)
this.instance.Dispose();
else
framework.RunOnFrameworkThread(() => this.instance.Dispose()).Wait();
this.instance = null;
}
this.DalamudInterface?.Dispose();
this.DalamudInterface = null;
this.ServiceScope?.Dispose();
this.ServiceScope = null;
this.pluginType = null;
this.pluginAssembly = null;
if (this.loader != null && didPluginDispose)
Thread.Sleep(configuration.PluginWaitBeforeFree ?? PluginManager.PluginWaitBeforeFreeDefault);
this.loader?.Dispose();
}
public virtual async ValueTask DisposeAsync() =>
await this.ClearAndDisposeAllResources(PluginLoaderDisposalMode.ImmediateDispose);
/// <summary>
/// Load this plugin.
@ -263,7 +238,6 @@ internal class LocalPlugin : IDisposable
/// <returns>A task.</returns>
public async Task LoadAsync(PluginLoadReason reason, bool reloading = false)
{
var framework = await Service<Framework>.GetAsync();
var ioc = await Service<ServiceContainer>.GetAsync();
var pluginManager = await Service<PluginManager>.GetAsync();
var dalamud = await Service<Dalamud>.GetAsync();
@ -300,11 +274,17 @@ internal class LocalPlugin : IDisposable
if (!this.IsDev)
{
throw new InvalidPluginOperationException(
$"Unable to load {this.Name}, unload previously faulted, restart Dalamud");
$"Unable to load {this.Name}, unload previously faulted, restart Dalamud");
}
break;
case PluginState.Unloaded:
if (this.instance is not null)
{
throw new InvalidPluginOperationException(
"Plugin should have been unloaded but instance is not cleared");
}
break;
case PluginState.Loading:
case PluginState.Unloading:
@ -406,39 +386,30 @@ internal class LocalPlugin : IDisposable
// NET8 CHORE
// PluginManager.PluginLocations[this.pluginType.Assembly.FullName] = new PluginPatchData(this.DllFile);
this.DalamudInterface =
new DalamudPluginInterface(this, reason);
this.dalamudInterface = new(this, reason);
this.ServiceScope = ioc.GetScope();
this.ServiceScope.RegisterPrivateScopes(this); // Add this LocalPlugin as a private scope, so services can get it
this.serviceScope = ioc.GetScope();
this.serviceScope.RegisterPrivateScopes(this); // Add this LocalPlugin as a private scope, so services can get it
try
{
var forceFrameworkThread = this.manifest.LoadSync && this.manifest.LoadRequiredState is 0 or 1;
var newInstanceTask = forceFrameworkThread ? framework.RunOnFrameworkThread(Create) : Create();
this.instance = await newInstanceTask.ConfigureAwait(false);
async Task<IDalamudPlugin> Create() =>
(IDalamudPlugin)await this.ServiceScope!.CreateAsync(this.pluginType!, this.DalamudInterface!);
this.instance = await CreatePluginInstance(
this.manifest,
this.serviceScope,
this.pluginType,
this.dalamudInterface);
this.State = PluginState.Loaded;
Log.Information("Finished loading {PluginName}", this.InternalName);
}
catch (Exception ex)
{
Log.Error(ex, "Exception during plugin initialization");
this.instance = null;
}
if (this.instance == null)
{
this.State = PluginState.LoadError;
this.UnloadAndDisposeState();
Log.Error(
"Error while loading {PluginName}, failed to bind and call the plugin constructor", this.InternalName);
return;
ex,
"Error while loading {PluginName}, failed to bind and call the plugin constructor",
this.InternalName);
await this.ClearAndDisposeAllResources(PluginLoaderDisposalMode.ImmediateDispose);
}
this.State = PluginState.Loaded;
Log.Information("Finished loading {PluginName}", this.InternalName);
}
catch (Exception ex)
{
@ -462,14 +433,10 @@ internal class LocalPlugin : IDisposable
/// Unload this plugin. This is the same as dispose, but without the "disposed" connotations. This object should stay
/// in the plugin list until it has been actually disposed.
/// </summary>
/// <param name="reloading">Unload while reloading.</param>
/// <param name="waitBeforeLoaderDispose">Wait before disposing loader.</param>
/// <param name="disposalMode">How to dispose loader.</param>
/// <returns>The task.</returns>
public async Task UnloadAsync(bool reloading = false, bool waitBeforeLoaderDispose = true)
public async Task UnloadAsync(PluginLoaderDisposalMode disposalMode = PluginLoaderDisposalMode.WaitBeforeDispose)
{
var configuration = Service<DalamudConfiguration>.Get();
var framework = Service<Framework>.GetNullable();
await this.pluginLoadStateLock.WaitAsync();
try
{
@ -498,31 +465,10 @@ internal class LocalPlugin : IDisposable
this.State = PluginState.Unloading;
Log.Information("Unloading {PluginName}", this.InternalName);
try
{
if (this.manifest.CanUnloadAsync || framework == null)
this.instance?.Dispose();
else
await framework.RunOnFrameworkThread(() => this.instance?.Dispose()).ConfigureAwait(false);
}
catch (Exception e)
if (await this.ClearAndDisposeAllResources(disposalMode) is { } ex)
{
this.State = PluginState.UnloadError;
Log.Error(e, "Could not unload {PluginName}, error in plugin dispose", this.InternalName);
return;
}
finally
{
this.instance = null;
this.UnloadAndDisposeState();
if (!reloading)
{
if (waitBeforeLoaderDispose && this.loader != null)
await Task.Delay(configuration.PluginWaitBeforeFree ?? PluginManager.PluginWaitBeforeFreeDefault);
this.loader?.Dispose();
this.loader = null;
}
throw ex;
}
this.State = PluginState.Unloaded;
@ -549,7 +495,7 @@ internal class LocalPlugin : IDisposable
{
// Don't unload if we're a dev plugin and have an unload error, this is a bad idea but whatever
if (this.IsDev && this.State != PluginState.UnloadError)
await this.UnloadAsync(true);
await this.UnloadAsync(PluginLoaderDisposalMode.None);
await this.LoadAsync(PluginLoadReason.Reload, true);
}
@ -617,6 +563,26 @@ internal class LocalPlugin : IDisposable
{
}
/// <summary>Creates a new instance of the plugin.</summary>
/// <param name="manifest">Plugin manifest.</param>
/// <param name="scope">Service scope.</param>
/// <param name="type">Type of the plugin main class.</param>
/// <param name="dalamudInterface">Instance of <see cref="IDalamudPluginInterface"/>.</param>
/// <returns>A new instance of the plugin.</returns>
private static async Task<IDalamudPlugin> CreatePluginInstance(
LocalPluginManifest manifest,
IServiceScope scope,
Type type,
DalamudPluginInterface dalamudInterface)
{
var framework = await Service<Framework>.GetAsync();
var forceFrameworkThread = manifest.LoadSync && manifest.LoadRequiredState is 0 or 1;
var newInstanceTask = forceFrameworkThread ? framework.RunOnFrameworkThread(Create) : Create();
return await newInstanceTask.ConfigureAwait(false);
async Task<IDalamudPlugin> Create() => (IDalamudPlugin)await scope.CreateAsync(type, dalamudInterface);
}
private static void SetupLoaderConfig(LoaderConfig config)
{
config.IsUnloadable = true;
@ -688,18 +654,110 @@ internal class LocalPlugin : IDisposable
}
}
private void UnloadAndDisposeState()
/// <summary>Clears and disposes all resources associated with the plugin instance.</summary>
/// <param name="disposalMode">Whether to clear and dispose <see cref="loader"/>.</param>
/// <returns>Exceptions, if any occurred.</returns>
private async Task<AggregateException?> ClearAndDisposeAllResources(PluginLoaderDisposalMode disposalMode)
{
if (this.instance != null)
throw new InvalidOperationException("Plugin instance should be disposed at this point");
List<Exception>? exceptions = null;
Log.Verbose(
"{name}({id}): {fn}(disposalMode={disposalMode})",
this.InternalName,
this.EffectiveWorkingPluginId,
nameof(this.ClearAndDisposeAllResources),
disposalMode);
this.DalamudInterface?.Dispose();
this.DalamudInterface = null;
this.ServiceScope?.Dispose();
this.ServiceScope = null;
// Clear the plugin instance first.
if (!await AttemptCleanup(
nameof(this.instance),
Interlocked.Exchange(ref this.instance, null),
this.manifest,
static async (inst, manifest) =>
{
var framework = Service<Framework>.GetNullable();
if (manifest.CanUnloadAsync || framework is null)
inst.Dispose();
else
await framework.RunOnFrameworkThread(inst.Dispose).ConfigureAwait(false);
}))
{
// Plugin was not loaded; loader is not referenced anyway, so no need to wait.
disposalMode = PluginLoaderDisposalMode.ImmediateDispose;
}
// Fields below are expected to be alive until the plugin is (attempted) disposed.
// Clear them after this point.
this.pluginType = null;
this.pluginAssembly = null;
await AttemptCleanup(
nameof(this.serviceScope),
Interlocked.Exchange(ref this.serviceScope, null),
0,
static (x, _) => x.DisposeAsync());
await AttemptCleanup(
nameof(this.dalamudInterface),
Interlocked.Exchange(ref this.dalamudInterface, null),
0,
static (x, _) =>
{
x.Dispose();
return ValueTask.CompletedTask;
});
if (disposalMode != PluginLoaderDisposalMode.None)
{
await AttemptCleanup(
nameof(this.loader),
Interlocked.Exchange(ref this.loader, null),
disposalMode == PluginLoaderDisposalMode.WaitBeforeDispose
? Service<DalamudConfiguration>.Get().PluginWaitBeforeFree ??
PluginManager.PluginWaitBeforeFreeDefault
: 0,
static async (ldr, waitBeforeDispose) =>
{
// Just in case plugins still have tasks running that they didn't cancel when they should have,
// give them some time to complete it.
// This helps avoid plugins being reloaded from conflicting with itself of previous instance.
await Task.Delay(waitBeforeDispose);
ldr.Dispose();
});
}
return exceptions is not null
? (AggregateException)ExceptionDispatchInfo.SetCurrentStackTrace(new AggregateException(exceptions))
: null;
async ValueTask<bool> AttemptCleanup<T, TContext>(
string name,
T? what,
TContext context,
Func<T, TContext, ValueTask> cb)
where T : class
{
if (what is null)
return false;
try
{
await cb.Invoke(what, context);
Log.Verbose("{name}({id}): {what} disposed", this.InternalName, this.EffectiveWorkingPluginId, name);
}
catch (Exception ex)
{
exceptions ??= [];
exceptions.Add(ex);
Log.Error(
ex,
"{name}({id}): Failed to dispose {what}",
this.InternalName,
this.EffectiveWorkingPluginId,
name);
}
return true;
}
}
}

19
Dalamud/Plugin/Internal/Types/PluginLoaderDisposalMode.cs Normal file
View file

@ -0,0 +1,19 @@
using System.Threading.Tasks;
using Dalamud.Plugin.Internal.Loader;
namespace Dalamud.Plugin.Internal.Types;
/// <summary>Specify how to dispose <see cref="PluginLoader"/>.</summary>
internal enum PluginLoaderDisposalMode
{
/// <summary>Do not dispose the plugin loader.</summary>
None,
/// <summary>Whether to wait a few before disposing the loader, just in case there are <see cref="Task{TResult}"/>s
/// from the plugin that are still running.</summary>
WaitBeforeDispose,
/// <summary>Immediately dispose the plugin loader.</summary>
ImmediateDispose,
}