[AddonEventManager] Actually Ensure Thread Safety (#1589)

* Actually make AddonEventManager thread safe

* Ensure AddonEventHandlers are also thread safe

Additionally, use Guid instead of strings

* Make DalamudInternalKey readonly

* Properly use ConcurrentDict features

Fixes GUID not working
This commit is contained in:
MidoriKami 2023-12-31 14:30:21 -08:00 committed by GitHub
parent a6b802b577
commit 02b1f6e426
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 58 deletions

View file

@ -67,7 +67,10 @@ internal unsafe class AddonEventListener : IDisposable
{ {
if (node is null) return; if (node is null) return;
node->AddEvent(eventType, param, this.eventListener, (AtkResNode*)addon, false); Service<Framework>.Get().RunOnFrameworkThread(() =>
{
node->AddEvent(eventType, param, this.eventListener, (AtkResNode*)addon, false);
});
} }
/// <summary> /// <summary>
@ -80,7 +83,10 @@ internal unsafe class AddonEventListener : IDisposable
{ {
if (node is null) return; if (node is null) return;
node->RemoveEvent(eventType, param, this.eventListener, false); Service<Framework>.Get().RunOnFrameworkThread(() =>
{
node->RemoveEvent(eventType, param, this.eventListener, false);
});
} }
[UnmanagedCallersOnly] [UnmanagedCallersOnly]

View file

@ -1,5 +1,4 @@
using System.Collections.Generic; using System.Collections.Concurrent;
using System.Linq;
using Dalamud.Game.Addon.Lifecycle; using Dalamud.Game.Addon.Lifecycle;
using Dalamud.Game.Addon.Lifecycle.AddonArgTypes; using Dalamud.Game.Addon.Lifecycle.AddonArgTypes;
@ -9,7 +8,6 @@ using Dalamud.IoC.Internal;
using Dalamud.Logging.Internal; using Dalamud.Logging.Internal;
using Dalamud.Plugin.Internal.Types; using Dalamud.Plugin.Internal.Types;
using Dalamud.Plugin.Services; using Dalamud.Plugin.Services;
using Dalamud.Utility;
using FFXIVClientStructs.FFXIV.Client.UI; using FFXIVClientStructs.FFXIV.Client.UI;
using FFXIVClientStructs.FFXIV.Component.GUI; using FFXIVClientStructs.FFXIV.Component.GUI;
@ -26,22 +24,19 @@ internal unsafe class AddonEventManager : IDisposable, IServiceType
/// <summary> /// <summary>
/// PluginName for Dalamud Internal use. /// PluginName for Dalamud Internal use.
/// </summary> /// </summary>
public const string DalamudInternalKey = "Dalamud.Internal"; public static readonly Guid DalamudInternalKey = Guid.NewGuid();
private static readonly ModuleLog Log = new("AddonEventManager"); private static readonly ModuleLog Log = new("AddonEventManager");
[ServiceManager.ServiceDependency] [ServiceManager.ServiceDependency]
private readonly AddonLifecycle addonLifecycle = Service<AddonLifecycle>.Get(); private readonly AddonLifecycle addonLifecycle = Service<AddonLifecycle>.Get();
[ServiceManager.ServiceDependency]
private readonly Framework framework = Service<Framework>.Get();
private readonly AddonLifecycleEventListener finalizeEventListener; private readonly AddonLifecycleEventListener finalizeEventListener;
private readonly AddonEventManagerAddressResolver address; private readonly AddonEventManagerAddressResolver address;
private readonly Hook<UpdateCursorDelegate> onUpdateCursor; private readonly Hook<UpdateCursorDelegate> onUpdateCursor;
private readonly List<PluginEventController> pluginEventControllers; private readonly ConcurrentDictionary<Guid, PluginEventController> pluginEventControllers;
private AddonCursorType? cursorOverride; private AddonCursorType? cursorOverride;
@ -51,10 +46,8 @@ internal unsafe class AddonEventManager : IDisposable, IServiceType
this.address = new AddonEventManagerAddressResolver(); this.address = new AddonEventManagerAddressResolver();
this.address.Setup(sigScanner); this.address.Setup(sigScanner);
this.pluginEventControllers = new List<PluginEventController> this.pluginEventControllers = new ConcurrentDictionary<Guid, PluginEventController>();
{ this.pluginEventControllers.TryAdd(DalamudInternalKey, new PluginEventController());
new(DalamudInternalKey), // Create entry for Dalamud's Internal Use.
};
this.cursorOverride = null; this.cursorOverride = null;
@ -73,7 +66,7 @@ internal unsafe class AddonEventManager : IDisposable, IServiceType
{ {
this.onUpdateCursor.Dispose(); this.onUpdateCursor.Dispose();
foreach (var pluginEventController in this.pluginEventControllers) foreach (var (_, pluginEventController) in this.pluginEventControllers)
{ {
pluginEventController.Dispose(); pluginEventController.Dispose();
} }
@ -90,16 +83,17 @@ internal unsafe class AddonEventManager : IDisposable, IServiceType
/// <param name="eventType">The event type for this event.</param> /// <param name="eventType">The event type for this event.</param>
/// <param name="eventHandler">The handler to call when event is triggered.</param> /// <param name="eventHandler">The handler to call when event is triggered.</param>
/// <returns>IAddonEventHandle used to remove the event.</returns> /// <returns>IAddonEventHandle used to remove the event.</returns>
internal IAddonEventHandle? AddEvent(string pluginId, IntPtr atkUnitBase, IntPtr atkResNode, AddonEventType eventType, IAddonEventManager.AddonEventHandler eventHandler) internal IAddonEventHandle? AddEvent(Guid pluginId, IntPtr atkUnitBase, IntPtr atkResNode, AddonEventType eventType, IAddonEventManager.AddonEventHandler eventHandler)
{ {
if (!ThreadSafety.IsMainThread) throw new InvalidOperationException("This should be done only from the main thread. Modifying active native code on non-main thread is not supported."); if (this.pluginEventControllers.TryGetValue(pluginId, out var controller))
if (this.pluginEventControllers.FirstOrDefault(entry => entry.PluginId == pluginId) is { } eventController)
{ {
return eventController.AddEvent(atkUnitBase, atkResNode, eventType, eventHandler); return controller.AddEvent(atkUnitBase, atkResNode, eventType, eventHandler);
}
else
{
Log.Verbose($"Unable to locate controller for {pluginId}. No event was added.");
} }
Log.Verbose($"Unable to locate controller for {pluginId}. No event was added.");
return null; return null;
} }
@ -108,13 +102,11 @@ internal unsafe class AddonEventManager : IDisposable, IServiceType
/// </summary> /// </summary>
/// <param name="pluginId">Unique ID for this plugin.</param> /// <param name="pluginId">Unique ID for this plugin.</param>
/// <param name="eventHandle">The Unique Id for this event.</param> /// <param name="eventHandle">The Unique Id for this event.</param>
internal void RemoveEvent(string pluginId, IAddonEventHandle eventHandle) internal void RemoveEvent(Guid pluginId, IAddonEventHandle eventHandle)
{ {
if (!ThreadSafety.IsMainThread) throw new InvalidOperationException("This should be done only from the main thread. Modifying active native code on non-main thread is not supported."); if (this.pluginEventControllers.TryGetValue(pluginId, out var controller))
if (this.pluginEventControllers.FirstOrDefault(entry => entry.PluginId == pluginId) is { } eventController)
{ {
eventController.RemoveEvent(eventHandle); controller.RemoveEvent(eventHandle);
} }
else else
{ {
@ -137,33 +129,28 @@ internal unsafe class AddonEventManager : IDisposable, IServiceType
/// Adds a new managed event controller if one doesn't already exist for this pluginId. /// Adds a new managed event controller if one doesn't already exist for this pluginId.
/// </summary> /// </summary>
/// <param name="pluginId">Unique ID for this plugin.</param> /// <param name="pluginId">Unique ID for this plugin.</param>
internal void AddPluginEventController(string pluginId) internal void AddPluginEventController(Guid pluginId)
{ {
this.framework.RunOnFrameworkThread(() => this.pluginEventControllers.GetOrAdd(
{ pluginId,
if (this.pluginEventControllers.All(entry => entry.PluginId != pluginId)) key =>
{ {
Log.Verbose($"Creating new PluginEventController for: {pluginId}"); Log.Verbose($"Creating new PluginEventController for: {key}");
this.pluginEventControllers.Add(new PluginEventController(pluginId)); return new PluginEventController();
} });
});
} }
/// <summary> /// <summary>
/// Removes an existing managed event controller for the specified plugin. /// Removes an existing managed event controller for the specified plugin.
/// </summary> /// </summary>
/// <param name="pluginId">Unique ID for this plugin.</param> /// <param name="pluginId">Unique ID for this plugin.</param>
internal void RemovePluginEventController(string pluginId) internal void RemovePluginEventController(Guid pluginId)
{ {
this.framework.RunOnFrameworkThread(() => if (this.pluginEventControllers.TryRemove(pluginId, out var controller))
{ {
if (this.pluginEventControllers.FirstOrDefault(entry => entry.PluginId == pluginId) is { } controller) Log.Verbose($"Removing PluginEventController for: {pluginId}");
{ controller.Dispose();
Log.Verbose($"Removing PluginEventController for: {pluginId}"); }
this.pluginEventControllers.Remove(controller);
controller.Dispose();
}
});
} }
/// <summary> /// <summary>
@ -178,7 +165,7 @@ internal unsafe class AddonEventManager : IDisposable, IServiceType
foreach (var pluginList in this.pluginEventControllers) foreach (var pluginList in this.pluginEventControllers)
{ {
pluginList.RemoveForAddon(addonInfo.AddonName); pluginList.Value.RemoveForAddon(addonInfo.AddonName);
} }
} }
@ -234,7 +221,7 @@ internal class AddonEventManagerPluginScoped : IDisposable, IServiceType, IAddon
{ {
this.plugin = plugin; this.plugin = plugin;
this.eventManagerService.AddPluginEventController(plugin.Manifest.WorkingPluginId.ToString()); this.eventManagerService.AddPluginEventController(plugin.Manifest.WorkingPluginId);
} }
/// <inheritdoc/> /// <inheritdoc/>
@ -246,16 +233,16 @@ internal class AddonEventManagerPluginScoped : IDisposable, IServiceType, IAddon
this.eventManagerService.ResetCursor(); this.eventManagerService.ResetCursor();
} }
this.eventManagerService.RemovePluginEventController(this.plugin.Manifest.WorkingPluginId.ToString()); this.eventManagerService.RemovePluginEventController(this.plugin.Manifest.WorkingPluginId);
} }
/// <inheritdoc/> /// <inheritdoc/>
public IAddonEventHandle? AddEvent(IntPtr atkUnitBase, IntPtr atkResNode, AddonEventType eventType, IAddonEventManager.AddonEventHandler eventHandler) public IAddonEventHandle? AddEvent(IntPtr atkUnitBase, IntPtr atkResNode, AddonEventType eventType, IAddonEventManager.AddonEventHandler eventHandler)
=> this.eventManagerService.AddEvent(this.plugin.Manifest.WorkingPluginId.ToString(), atkUnitBase, atkResNode, eventType, eventHandler); => this.eventManagerService.AddEvent(this.plugin.Manifest.WorkingPluginId, atkUnitBase, atkResNode, eventType, eventHandler);
/// <inheritdoc/> /// <inheritdoc/>
public void RemoveEvent(IAddonEventHandle eventHandle) public void RemoveEvent(IAddonEventHandle eventHandle)
=> this.eventManagerService.RemoveEvent(this.plugin.Manifest.WorkingPluginId.ToString(), eventHandle); => this.eventManagerService.RemoveEvent(this.plugin.Manifest.WorkingPluginId, eventHandle);
/// <inheritdoc/> /// <inheritdoc/>
public void SetCursor(AddonCursorType cursor) public void SetCursor(AddonCursorType cursor)

View file

@ -19,19 +19,11 @@ internal unsafe class PluginEventController : IDisposable
/// <summary> /// <summary>
/// Initializes a new instance of the <see cref="PluginEventController"/> class. /// Initializes a new instance of the <see cref="PluginEventController"/> class.
/// </summary> /// </summary>
/// <param name="pluginId">The Unique ID for this plugin.</param> public PluginEventController()
public PluginEventController(string pluginId)
{ {
this.PluginId = pluginId;
this.EventListener = new AddonEventListener(this.PluginEventListHandler); this.EventListener = new AddonEventListener(this.PluginEventListHandler);
} }
/// <summary>
/// Gets the unique ID for this PluginEventList.
/// </summary>
public string PluginId { get; init; }
private AddonEventListener EventListener { get; init; } private AddonEventListener EventListener { get; init; }
private List<AddonEventEntry> Events { get; } = new(); private List<AddonEventEntry> Events { get; } = new();
@ -125,7 +117,7 @@ internal unsafe class PluginEventController : IDisposable
if (this.Events.All(registeredEvent => registeredEvent.ParamKey != i)) return i; if (this.Events.All(registeredEvent => registeredEvent.ParamKey != i)) return i;
} }
throw new OverflowException($"uint.MaxValue number of ParamKeys used for {this.PluginId}"); throw new OverflowException($"uint.MaxValue number of ParamKeys used for this event controller.");
} }
/// <summary> /// <summary>