Add IInternal/PublicDisposableService (#1696)

* Add IInternal/PublicDisposableService

Plugins are exposed interfaces that are not inherited from
`IDisposable`, but services implementing plugin interfaces often
implement `IDisposable`. Some plugins may try to call
`IDisposable.Dispose` on everything provided, and it also is possible to
use `using` clause too eagerly while working on Dalamud itself, such as
writing `using var smth = await Service<SomeService>.GetAsync();`. Such
behaviors often lead to a difficult-to-debug errors, and making those
services either not an `IDisposable` or making `IDisposable.Dispose` do
nothing if the object has been loaded would prevent such errors. As
`ServiceManager` must be the only class dealing with construction and
disposal of services, `IInternalDisposableService` has been added to
limit who can dispose the object. `IPublicDisposableService` also has
been added to classes that can be constructed and accessed directly by
plugins; for those, `Dispose` will be ignored if the instance is a
service instance, and only `DisposeService` will respond.

In addition, `DalamudPluginInterface` and `UiBuilder` also have been
changed so that their `IDisposable.Dispose` no longer respond, and
instead, internal functions have been added to only allow disposal from
Dalamud.

* Cleanup

* Postmerge fixes

* More explanation on RunOnFrameworkThread(ClearHooks)

* Mark ReliableFileStorage public ctor obsolete

---------

Co-authored-by: goat <16760685+goaaats@users.noreply.github.com>
This commit is contained in:
srkizer 2024-03-17 00:58:05 +09:00 committed by GitHub
parent dcec076ca7
commit 87b9edb448
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
62 changed files with 441 additions and 381 deletions

View file

@ -452,26 +452,28 @@ public sealed class DalamudPluginInterface : IDisposable
#endregion
/// <summary>
/// Unregister your plugin and dispose all references.
/// </summary>
/// <inheritdoc cref="Dispose"/>
void IDisposable.Dispose()
{
this.UiBuilder.ExplicitDispose();
Service<ChatGui>.Get().RemoveChatLinkHandler(this.plugin.InternalName);
Service<Localization>.Get().LocalizationChanged -= this.OnLocalizationChanged;
Service<DalamudConfiguration>.Get().DalamudConfigurationSaved -= this.OnDalamudConfigurationSaved;
}
/// <summary>
/// Obsolete implicit dispose implementation. Should not be used.
/// </summary>
[Obsolete("Do not dispose \"DalamudPluginInterface\".", true)]
/// <summary>This function will do nothing. Dalamud will dispose this object on plugin unload.</summary>
[Obsolete("This function will do nothing. Dalamud will dispose this object on plugin unload.", true)]
public void Dispose()
{
// ignored
}
/// <summary>Unregister the plugin and dispose all references.</summary>
/// <remarks>Dalamud internal use only.</remarks>
internal void DisposeInternal()
{
Service<ChatGui>.Get().RemoveChatLinkHandler(this.plugin.InternalName);
Service<Localization>.Get().LocalizationChanged -= this.OnLocalizationChanged;
Service<DalamudConfiguration>.Get().DalamudConfigurationSaved -= this.OnDalamudConfigurationSaved;
this.UiBuilder.DisposeInternal();
}
/// <summary>
/// Dispatch the active plugins changed event.
/// </summary>

View file

@ -55,7 +55,7 @@ namespace Dalamud.Plugin.Internal;
[InherentDependency<DataShare>]
#pragma warning restore SA1015
internal partial class PluginManager : IDisposable, IServiceType
internal partial class PluginManager : IInternalDisposableService
{
/// <summary>
/// Default time to wait between plugin unload and plugin assembly unload.
@ -370,7 +370,7 @@ internal partial class PluginManager : IDisposable, IServiceType
}
/// <inheritdoc/>
public void Dispose()
void IInternalDisposableService.DisposeService()
{
var disposablePlugins =
this.installedPluginsList.Where(plugin => plugin.State is PluginState.Loaded or PluginState.LoadError).ToArray();
@ -410,7 +410,16 @@ internal partial class PluginManager : IDisposable, IServiceType
// 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)
plugin.ExplicitDisposeIgnoreExceptions($"Error disposing {plugin.Name}", Log);
{
try
{
plugin.Dispose();
}
catch (Exception e)
{
Log.Error(e, $"Error disposing {plugin.Name}");
}
}
}
this.assemblyLocationMonoHook?.Dispose();

View file

@ -16,7 +16,7 @@ namespace Dalamud.Plugin.Internal.Profiles;
/// Service responsible for profile-related chat commands.
/// </summary>
[ServiceManager.EarlyLoadedService]
internal class ProfileCommandHandler : IServiceType, IDisposable
internal class ProfileCommandHandler : IInternalDisposableService
{
private readonly CommandManager cmd;
private readonly ProfileManager profileManager;
@ -69,7 +69,7 @@ internal class ProfileCommandHandler : IServiceType, IDisposable
}
/// <inheritdoc/>
public void Dispose()
void IInternalDisposableService.DisposeService()
{
this.cmd.RemoveHandler("/xlenablecollection");
this.cmd.RemoveHandler("/xldisablecollection");

View file

@ -240,7 +240,7 @@ internal class LocalPlugin : IDisposable
this.instance = null;
}
this.DalamudInterface?.ExplicitDispose();
this.DalamudInterface?.DisposeInternal();
this.DalamudInterface = null;
this.ServiceScope?.Dispose();
@ -426,7 +426,7 @@ internal class LocalPlugin : IDisposable
if (this.instance == null)
{
this.State = PluginState.LoadError;
this.DalamudInterface.ExplicitDispose();
this.DalamudInterface.DisposeInternal();
Log.Error(
$"Error while loading {this.Name}, failed to bind and call the plugin constructor");
return;
@ -499,7 +499,7 @@ internal class LocalPlugin : IDisposable
this.instance = null;
this.DalamudInterface?.ExplicitDispose();
this.DalamudInterface?.DisposeInternal();
this.DalamudInterface = null;
this.ServiceScope?.Dispose();