fix: lock profile manager while changing state

This commit is contained in:
goat 2023-06-20 19:20:08 +02:00
parent 3d47e05ab0
commit 28e9d5f156
No known key found for this signature in database
GPG key ID: 49E2AA8C6A76498B
5 changed files with 77 additions and 43 deletions

View file

@ -2898,7 +2898,7 @@ internal class PluginInstallerWindow : Window, IDisposable
private void OnInstalledPluginsChanged() private void OnInstalledPluginsChanged()
{ {
var pluginManager = Service<PluginManager>.Get(); var pluginManager = Service<PluginManager>.Get();
using var pmLock = pluginManager.LockPluginLists(); using var pmLock = pluginManager.GetSyncScope();
lock (this.listLock) lock (this.listLock)
{ {

View file

@ -264,7 +264,7 @@ internal partial class PluginManager : IDisposable, IServiceType
/// Get a disposable that will lock plugin lists while it is not disposed. /// Get a disposable that will lock plugin lists while it is not disposed.
/// </summary> /// </summary>
/// <returns>The aforementioned disposable.</returns> /// <returns>The aforementioned disposable.</returns>
public IDisposable LockPluginLists() => new PluginListLockScope(this.pluginListLock); public IDisposable GetSyncScope() => new ScopedSyncRoot(this.pluginListLock);
/// <summary> /// <summary>
/// Print to chat any plugin updates and whether they were successful. /// Print to chat any plugin updates and whether they were successful.
@ -1560,25 +1560,4 @@ internal partial class PluginManager
var codebasePatch = typeof(PluginManager).GetMethod(nameof(AssemblyCodeBasePatch), BindingFlags.NonPublic | BindingFlags.Static); var codebasePatch = typeof(PluginManager).GetMethod(nameof(AssemblyCodeBasePatch), BindingFlags.NonPublic | BindingFlags.Static);
this.assemblyCodeBaseMonoHook = new MonoMod.RuntimeDetour.Hook(codebaseTarget, codebasePatch); this.assemblyCodeBaseMonoHook = new MonoMod.RuntimeDetour.Hook(codebaseTarget, codebasePatch);
} }
/// <summary>
/// Scope for plugin list locks.
/// </summary>
private class PluginListLockScope : IDisposable
{
private readonly object lockObj;
public PluginListLockScope(object lockObj)
{
this.lockObj = lockObj;
Monitor.Enter(lockObj);
}
/// <inheritdoc/>
public void Dispose()
{
GC.SuppressFinalize(this);
Monitor.Exit(this.lockObj);
}
}
} }

View file

@ -137,6 +137,8 @@ internal class Profile
/// <returns>Null if this profile does not declare the plugin, true if the profile declares the plugin and wants it enabled, false if the profile declares the plugin and does not want it enabled.</returns> /// <returns>Null if this profile does not declare the plugin, true if the profile declares the plugin and wants it enabled, false if the profile declares the plugin and does not want it enabled.</returns>
public bool? WantsPlugin(string internalName) public bool? WantsPlugin(string internalName)
{ {
using var lockScope = this.manager.GetSyncScope();
var entry = this.modelV1.Plugins.FirstOrDefault(x => x.InternalName == internalName); var entry = this.modelV1.Plugins.FirstOrDefault(x => x.InternalName == internalName);
return entry?.IsEnabled; return entry?.IsEnabled;
} }
@ -150,6 +152,8 @@ internal class Profile
/// <param name="apply">Whether or not the current state should immediately be applied.</param> /// <param name="apply">Whether or not the current state should immediately be applied.</param>
public void AddOrUpdate(string internalName, bool state, bool apply = true) public void AddOrUpdate(string internalName, bool state, bool apply = true)
{ {
using var lockScope = this.manager.GetSyncScope();
Debug.Assert(!internalName.IsNullOrEmpty(), "!internalName.IsNullOrEmpty()"); Debug.Assert(!internalName.IsNullOrEmpty(), "!internalName.IsNullOrEmpty()");
var existing = this.modelV1.Plugins.FirstOrDefault(x => x.InternalName == internalName); var existing = this.modelV1.Plugins.FirstOrDefault(x => x.InternalName == internalName);
@ -186,6 +190,8 @@ internal class Profile
/// <param name="apply">Whether or not the current state should immediately be applied.</param> /// <param name="apply">Whether or not the current state should immediately be applied.</param>
public void Remove(string internalName, bool apply = true) public void Remove(string internalName, bool apply = true)
{ {
using var lockScope = this.manager.GetSyncScope();
var entry = this.modelV1.Plugins.FirstOrDefault(x => x.InternalName == internalName); var entry = this.modelV1.Plugins.FirstOrDefault(x => x.InternalName == internalName);
if (entry == null) if (entry == null)
throw new ArgumentException($"No plugin \"{internalName}\" in profile \"{this.Guid}\""); throw new ArgumentException($"No plugin \"{internalName}\" in profile \"{this.Guid}\"");

View file

@ -41,7 +41,14 @@ internal class ProfileManager : IServiceType
/// <summary> /// <summary>
/// Gets the default profile. /// Gets the default profile.
/// </summary> /// </summary>
public Profile DefaultProfile => this.profiles.First(x => x.IsDefaultProfile); public Profile DefaultProfile
{
get
{
lock (this.profiles)
return this.profiles.First(x => x.IsDefaultProfile);
}
}
/// <summary> /// <summary>
/// Gets all profiles, including the default profile. /// Gets all profiles, including the default profile.
@ -53,6 +60,12 @@ internal class ProfileManager : IServiceType
/// </summary> /// </summary>
public bool IsBusy => this.isBusy; public bool IsBusy => this.isBusy;
/// <summary>
/// Get a disposable that will lock the profile list while it is not disposed.
/// </summary>
/// <returns>The aforementioned disposable.</returns>
public ScopedSyncRoot GetSyncScope() => new ScopedSyncRoot(this.profiles);
/// <summary> /// <summary>
/// Check if any enabled profile wants a specific plugin enabled. /// Check if any enabled profile wants a specific plugin enabled.
/// </summary> /// </summary>
@ -62,27 +75,30 @@ internal class ProfileManager : IServiceType
/// <returns>Whether or not the plugin shall be enabled.</returns> /// <returns>Whether or not the plugin shall be enabled.</returns>
public bool GetWantState(string internalName, bool defaultState, bool addIfNotDeclared = true) public bool GetWantState(string internalName, bool defaultState, bool addIfNotDeclared = true)
{ {
var want = false; lock (this.profiles)
var wasInAnyProfile = false;
foreach (var profile in this.profiles)
{ {
var state = profile.WantsPlugin(internalName); var want = false;
if (state.HasValue) var wasInAnyProfile = false;
foreach (var profile in this.profiles)
{ {
want = want || (profile.IsEnabled && state.Value); var state = profile.WantsPlugin(internalName);
wasInAnyProfile = true; if (state.HasValue)
{
want = want || (profile.IsEnabled && state.Value);
wasInAnyProfile = true;
}
} }
}
if (!wasInAnyProfile && addIfNotDeclared) if (!wasInAnyProfile && addIfNotDeclared)
{ {
Log.Warning("{Name} was not in any profile, adding to default with {Default}", internalName, defaultState); Log.Warning("{Name} was not in any profile, adding to default with {Default}", internalName, defaultState);
this.DefaultProfile.AddOrUpdate(internalName, defaultState, false); this.DefaultProfile.AddOrUpdate(internalName, defaultState, false);
return defaultState; return defaultState;
} }
return want; return want;
}
} }
/// <summary> /// <summary>
@ -91,7 +107,10 @@ internal class ProfileManager : IServiceType
/// <param name="internalName">The internal name of the plugin.</param> /// <param name="internalName">The internal name of the plugin.</param>
/// <returns>Whether or not the plugin is in any profile.</returns> /// <returns>Whether or not the plugin is in any profile.</returns>
public bool IsInAnyProfile(string internalName) public bool IsInAnyProfile(string internalName)
=> this.profiles.Any(x => x.WantsPlugin(internalName) != null); {
lock (this.profiles)
return this.profiles.Any(x => x.WantsPlugin(internalName) != null);
}
/// <summary> /// <summary>
/// Check whether a plugin is only in the default profile. /// Check whether a plugin is only in the default profile.
@ -170,8 +189,9 @@ internal class ProfileManager : IServiceType
public void ApplyAllWantStates() public void ApplyAllWantStates()
{ {
var pm = Service<PluginManager>.Get(); var pm = Service<PluginManager>.Get();
using var pmLock = pm.LockPluginLists(); using var managerLock = pm.GetSyncScope();
using var profilesLock = this.GetSyncScope();
this.isBusy = true; this.isBusy = true;
Log.Information("Getting want states..."); Log.Information("Getting want states...");

View file

@ -0,0 +1,29 @@
using System;
using System.Threading;
namespace Dalamud.Utility;
/// <summary>
/// Scope for plugin list locks.
/// </summary>
public class ScopedSyncRoot : IDisposable
{
private readonly object lockObj;
/// <summary>
/// Initializes a new instance of the <see cref="ScopedSyncRoot"/> class.
/// </summary>
/// <param name="lockObj">The object to lock.</param>
public ScopedSyncRoot(object lockObj)
{
this.lockObj = lockObj;
Monitor.Enter(lockObj);
}
/// <inheritdoc/>
public void Dispose()
{
GC.SuppressFinalize(this);
Monitor.Exit(this.lockObj);
}
}