diff --git a/Dalamud/Game/ChatHandlers.cs b/Dalamud/Game/ChatHandlers.cs index 7cd0869aa..ed69b7bbe 100644 --- a/Dalamud/Game/ChatHandlers.cs +++ b/Dalamud/Game/ChatHandlers.cs @@ -292,7 +292,7 @@ public class ChatHandlers : IServiceType if (chatGui == null || pluginManager == null || notifications == null) return; - if (!pluginManager.ReposReady || pluginManager.InstalledPlugins.Count == 0 || pluginManager.AvailablePlugins.Count == 0) + if (!pluginManager.ReposReady || !pluginManager.InstalledPlugins.Any() || !pluginManager.AvailablePlugins.Any()) { // Plugins aren't ready yet. // TODO: We should retry. This sucks, because it means we won't ever get here again until another notice. @@ -311,7 +311,7 @@ public class ChatHandlers : IServiceType return; } - var updatedPlugins = task.Result; + var updatedPlugins = task.Result.ToList(); if (updatedPlugins.Any()) { if (this.configuration.AutoUpdatePlugins) diff --git a/Dalamud/Interface/Internal/DalamudInterface.cs b/Dalamud/Interface/Internal/DalamudInterface.cs index ca7cbe287..caf38d4d7 100644 --- a/Dalamud/Interface/Internal/DalamudInterface.cs +++ b/Dalamud/Interface/Internal/DalamudInterface.cs @@ -900,7 +900,7 @@ internal class DalamudInterface : IDisposable, IServiceType ImGui.Separator(); ImGui.MenuItem("API Level:" + PluginManager.DalamudApiLevel, false); - ImGui.MenuItem("Loaded plugins:" + pluginManager.InstalledPlugins.Count, false); + ImGui.MenuItem("Loaded plugins:" + pluginManager.InstalledPlugins.Count(), false); ImGui.EndMenu(); } diff --git a/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs b/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs index 33b80cf93..af642d9cf 100644 --- a/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs +++ b/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs @@ -2898,6 +2898,7 @@ internal class PluginInstallerWindow : Window, IDisposable private void OnInstalledPluginsChanged() { var pluginManager = Service.Get(); + using var pmLock = pluginManager.LockPluginLists(); lock (this.listLock) { diff --git a/Dalamud/Plugin/Internal/PluginManager.cs b/Dalamud/Plugin/Internal/PluginManager.cs index 92026cf2a..907814123 100644 --- a/Dalamud/Plugin/Internal/PluginManager.cs +++ b/Dalamud/Plugin/Internal/PluginManager.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Collections.Immutable; using System.Diagnostics; using System.IO; using System.IO.Compression; @@ -68,6 +67,10 @@ internal partial class PluginManager : IDisposable, IServiceType private readonly object pluginListLock = new(); private readonly DirectoryInfo pluginDirectory; private readonly BannedPlugin[]? bannedPlugins; + + private readonly List installedPluginsList = new(); + private readonly List availablePluginsList = new(); + private readonly List updatablePluginsList = new(); private readonly DalamudLinkPayload openInstallerWindowPluginChangelogsLink; @@ -145,19 +148,46 @@ internal partial class PluginManager : IDisposable, IServiceType public event Action? OnAvailablePluginsChanged; /// - /// Gets a list of all loaded plugins. + /// Gets a copy of the list of all loaded plugins. /// - public ImmutableList InstalledPlugins { get; private set; } = ImmutableList.Create(); + public IEnumerable InstalledPlugins + { + get + { + lock (this.pluginListLock) + { + return this.installedPluginsList.ToList(); + } + } + } /// /// Gets a list of all available plugins. /// - public ImmutableList AvailablePlugins { get; private set; } = ImmutableList.Create(); - + public IEnumerable AvailablePlugins + { + get + { + lock (this.pluginListLock) + { + return this.availablePluginsList.ToList(); + } + } + } + /// /// Gets a list of all plugins with an available update. /// - public ImmutableList UpdatablePlugins { get; private set; } = ImmutableList.Create(); + public IEnumerable UpdatablePlugins + { + get + { + lock (this.pluginListLock) + { + return this.updatablePluginsList.ToList(); + } + } + } /// /// Gets a list of all plugin repositories. The main repo should always be first. @@ -230,6 +260,12 @@ internal partial class PluginManager : IDisposable, IServiceType return false; } + /// + /// Get a disposable that will lock plugin lists while it is not disposed. + /// + /// The aforementioned disposable. + public IDisposable LockPluginLists() => new PluginListLockScope(this.pluginListLock); + /// /// Print to chat any plugin updates and whether they were successful. /// @@ -308,7 +344,7 @@ internal partial class PluginManager : IDisposable, IServiceType public void Dispose() { var disposablePlugins = - this.InstalledPlugins.Where(plugin => plugin.State is PluginState.Loaded or PluginState.LoadError).ToArray(); + 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. @@ -581,7 +617,7 @@ internal partial class PluginManager : IDisposable, IServiceType var sigScanner = await Service.GetAsync().ConfigureAwait(false); this.PluginsReady = true; - this.NotifyInstalledPluginsChanged(); + this.NotifyinstalledPluginsListChanged(); sigScanner.Save(); }, tokenSource.Token); @@ -596,7 +632,7 @@ internal partial class PluginManager : IDisposable, IServiceType { lock (this.pluginListLock) { - return Task.WhenAll(this.InstalledPlugins + return Task.WhenAll(this.installedPluginsList .Where(x => x.IsLoaded) .ToList() .Select(x => Task.Run(async () => await x.ReloadAsync())) @@ -629,11 +665,14 @@ internal partial class PluginManager : IDisposable, IServiceType /// Whether to notify that available plugins have changed afterwards. public void RefilterPluginMasters(bool notify = true) { - this.AvailablePlugins = this.Repos - .SelectMany(repo => repo.PluginMaster) - .Where(this.IsManifestEligible) - .Where(IsManifestVisible) - .ToImmutableList(); + lock (this.pluginListLock) + { + this.availablePluginsList.Clear(); + this.availablePluginsList.AddRange(this.Repos + .SelectMany(repo => repo.PluginMaster) + .Where(this.IsManifestEligible) + .Where(IsManifestVisible)); + } if (notify) { @@ -673,7 +712,7 @@ internal partial class PluginManager : IDisposable, IServiceType // This file is already known to us lock (this.pluginListLock) { - if (this.InstalledPlugins.Any(lp => lp.DllFile.FullName == dllFile.FullName)) + if (this.installedPluginsList.Any(lp => lp.DllFile.FullName == dllFile.FullName)) continue; } @@ -699,7 +738,7 @@ internal partial class PluginManager : IDisposable, IServiceType } if (listChanged) - this.NotifyInstalledPluginsChanged(); + this.NotifyinstalledPluginsListChanged(); } /// @@ -817,7 +856,7 @@ internal partial class PluginManager : IDisposable, IServiceType var plugin = await this.LoadPluginAsync(dllFile, manifest, reason); - this.NotifyInstalledPluginsChanged(); + this.NotifyinstalledPluginsListChanged(); return plugin; } @@ -832,12 +871,12 @@ internal partial class PluginManager : IDisposable, IServiceType lock (this.pluginListLock) { - this.InstalledPlugins = this.InstalledPlugins.Remove(plugin); + this.installedPluginsList.Remove(plugin); } PluginLocations.Remove(plugin.AssemblyName?.FullName ?? string.Empty, out _); - this.NotifyInstalledPluginsChanged(); + this.NotifyinstalledPluginsListChanged(); this.NotifyAvailablePluginsChanged(); } @@ -937,31 +976,34 @@ internal partial class PluginManager : IDisposable, IServiceType /// Perform a dry run, don't install anything. /// If this action was performed as part of an auto-update. /// Success or failure and a list of updated plugin metadata. - public async Task> UpdatePluginsAsync(bool ignoreDisabled, bool dryRun, bool autoUpdate = false) + public async Task> UpdatePluginsAsync(bool ignoreDisabled, bool dryRun, bool autoUpdate = false) { Log.Information("Starting plugin update"); - var updatedList = new List(); + var updateTasks = new List>(); // Prevent collection was modified errors - foreach (var plugin in this.UpdatablePlugins) + lock (this.pluginListLock) { - // Can't update that! - if (plugin.InstalledPlugin.IsDev) - continue; + foreach (var plugin in this.updatablePluginsList) + { + // Can't update that! + if (plugin.InstalledPlugin.IsDev) + continue; - if (!plugin.InstalledPlugin.IsWantedByAnyProfile && ignoreDisabled) - continue; + if (!plugin.InstalledPlugin.IsWantedByAnyProfile && ignoreDisabled) + continue; - if (plugin.InstalledPlugin.Manifest.ScheduledForDeletion) - continue; + if (plugin.InstalledPlugin.Manifest.ScheduledForDeletion) + continue; - var result = await this.UpdateSinglePluginAsync(plugin, false, dryRun); - if (result != null) - updatedList.Add(result); + updateTasks.Add(this.UpdateSinglePluginAsync(plugin, false, dryRun)); + } } - this.NotifyInstalledPluginsChanged(); + var updatedList = await Task.WhenAll(updateTasks); + + this.NotifyinstalledPluginsListChanged(); this.NotifyPluginsForStateChange( autoUpdate ? PluginListInvalidationKind.AutoUpdate : PluginListInvalidationKind.Update, updatedList.Select(x => x.InternalName)); @@ -978,7 +1020,7 @@ internal partial class PluginManager : IDisposable, IServiceType /// Whether to notify that installed plugins have changed afterwards. /// Whether or not to actually perform the update, or just indicate success. /// The status of the update. - public async Task UpdateSinglePluginAsync(AvailablePluginUpdate metadata, bool notify, bool dryRun) + public async Task UpdateSinglePluginAsync(AvailablePluginUpdate metadata, bool notify, bool dryRun) { var plugin = metadata.InstalledPlugin; @@ -1025,7 +1067,7 @@ internal partial class PluginManager : IDisposable, IServiceType lock (this.pluginListLock) { - this.InstalledPlugins = this.InstalledPlugins.Remove(plugin); + this.installedPluginsList.Remove(plugin); } } catch (Exception ex) @@ -1052,7 +1094,7 @@ internal partial class PluginManager : IDisposable, IServiceType } if (notify && updateStatus.WasUpdated) - this.NotifyInstalledPluginsChanged(); + this.NotifyinstalledPluginsListChanged(); return updateStatus; } @@ -1184,7 +1226,7 @@ internal partial class PluginManager : IDisposable, IServiceType lock (this.pluginListLock) { - foreach (var plugin in this.InstalledPlugins) + foreach (var plugin in this.installedPluginsList) { if (plugin.AssemblyName != null && plugin.AssemblyName.FullName == declaringType.Assembly.GetName().FullName) @@ -1219,11 +1261,11 @@ internal partial class PluginManager : IDisposable, IServiceType var name = manifest?.Name ?? dllFile.Name; var loadPlugin = !doNotLoad; - LocalPlugin plugin; + LocalPlugin? plugin; - if (manifest != null && manifest.InternalName == null) + if (manifest != null && (manifest.InternalName == null || manifest.Name == null)) { - Log.Error("{FileName}: Your manifest has no internal name set! Can't load this.", dllFile.FullName); + Log.Error("{FileName}: Your manifest has no internal name or name set! Can't load this.", dllFile.FullName); throw new Exception("No internal name"); } @@ -1327,9 +1369,12 @@ internal partial class PluginManager : IDisposable, IServiceType } } + if (plugin == null) + throw new Exception("Plugin was null when adding to list"); + lock (this.pluginListLock) { - this.InstalledPlugins = this.InstalledPlugins.Add(plugin); + this.installedPluginsList.Add(plugin); } return plugin; @@ -1337,39 +1382,40 @@ internal partial class PluginManager : IDisposable, IServiceType private void DetectAvailablePluginUpdates() { - var updatablePlugins = new List(); - - foreach (var plugin in this.InstalledPlugins) + lock (this.pluginListLock) { - var installedVersion = plugin.IsTesting - ? plugin.Manifest.TestingAssemblyVersion - : plugin.Manifest.AssemblyVersion; - - var updates = this.AvailablePlugins - .Where(remoteManifest => plugin.Manifest.InternalName == remoteManifest.InternalName) - .Where(remoteManifest => plugin.Manifest.InstalledFromUrl == remoteManifest.SourceRepo.PluginMasterUrl || !remoteManifest.SourceRepo.IsThirdParty) - .Where(remoteManifest => remoteManifest.DalamudApiLevel == DalamudApiLevel) - .Select(remoteManifest => - { - var useTesting = this.UseTesting(remoteManifest); - var candidateVersion = useTesting - ? remoteManifest.TestingAssemblyVersion - : remoteManifest.AssemblyVersion; - var isUpdate = candidateVersion > installedVersion; - - return (isUpdate, useTesting, candidateVersion, remoteManifest); - }) - .Where(tpl => tpl.isUpdate) - .ToList(); - - if (updates.Count > 0) + this.updatablePluginsList.Clear(); + + foreach (var plugin in this.installedPluginsList) { - var update = updates.Aggregate((t1, t2) => t1.candidateVersion > t2.candidateVersion ? t1 : t2); - updatablePlugins.Add(new AvailablePluginUpdate(plugin, update.remoteManifest, update.useTesting)); + var installedVersion = plugin.IsTesting + ? plugin.Manifest.TestingAssemblyVersion + : plugin.Manifest.AssemblyVersion; + + var updates = this.AvailablePlugins + .Where(remoteManifest => plugin.Manifest.InternalName == remoteManifest.InternalName) + .Where(remoteManifest => plugin.Manifest.InstalledFromUrl == remoteManifest.SourceRepo.PluginMasterUrl || !remoteManifest.SourceRepo.IsThirdParty) + .Where(remoteManifest => remoteManifest.DalamudApiLevel == DalamudApiLevel) + .Select(remoteManifest => + { + var useTesting = this.UseTesting(remoteManifest); + var candidateVersion = useTesting + ? remoteManifest.TestingAssemblyVersion + : remoteManifest.AssemblyVersion; + var isUpdate = candidateVersion > installedVersion; + + return (isUpdate, useTesting, candidateVersion, remoteManifest); + }) + .Where(tpl => tpl.isUpdate) + .ToList(); + + if (updates.Count > 0) + { + var update = updates.Aggregate((t1, t2) => t1.candidateVersion > t2.candidateVersion ? t1 : t2); + this.updatablePluginsList.Add(new AvailablePluginUpdate(plugin, update.remoteManifest, update.useTesting)); + } } } - - this.UpdatablePlugins = updatablePlugins.ToImmutableList(); } private void NotifyAvailablePluginsChanged() @@ -1379,7 +1425,7 @@ internal partial class PluginManager : IDisposable, IServiceType this.OnAvailablePluginsChanged?.InvokeSafely(); } - private void NotifyInstalledPluginsChanged() + private void NotifyinstalledPluginsListChanged() { this.DetectAvailablePluginUpdates(); @@ -1388,7 +1434,7 @@ internal partial class PluginManager : IDisposable, IServiceType private void NotifyPluginsForStateChange(PluginListInvalidationKind kind, IEnumerable affectedInternalNames) { - foreach (var installedPlugin in this.InstalledPlugins) + foreach (var installedPlugin in this.installedPluginsList) { if (!installedPlugin.IsLoaded || installedPlugin.DalamudInterface == null) continue; @@ -1514,4 +1560,25 @@ internal partial class PluginManager var codebasePatch = typeof(PluginManager).GetMethod(nameof(AssemblyCodeBasePatch), BindingFlags.NonPublic | BindingFlags.Static); this.assemblyCodeBaseMonoHook = new MonoMod.RuntimeDetour.Hook(codebaseTarget, codebasePatch); } + + /// + /// Scope for plugin list locks. + /// + private class PluginListLockScope : IDisposable + { + private readonly object lockObj; + + public PluginListLockScope(object lockObj) + { + this.lockObj = lockObj; + Monitor.Enter(lockObj); + } + + /// + public void Dispose() + { + GC.SuppressFinalize(this); + Monitor.Exit(this.lockObj); + } + } }