Accommodate nested AddonLifecycle event calls (#1698)

* Accommodate nested AddonLifecycle event calls

The game is free to call event handlers of another addon from one addon,
but the previous code was written under the assumption that only one
function may be called at a time. This changes the recycled addon args
into pooled args.

* Always clear addon name cache

Dalamud/Dalamud.csproj

@@ -112,10 +112,6 @@
       </Content>
     </ItemGroup>
 
-    <ItemGroup>
-      <Folder Include="Game\Addon\" />
-    </ItemGroup>
-
     <Target Name="AddRuntimeDependenciesToContent" BeforeTargets="GetCopyToOutputDirectoryItems" DependsOnTargets="GenerateBuildDependencyFile;GenerateBuildRuntimeConfigurationFiles">
         <ItemGroup>
             <ContentWithTargetPath Include="$(ProjectDepsFilePath)" CopyToOutputDirectory="PreserveNewest" TargetPath="$(ProjectDepsFileName)" />

Dalamud/Game/Addon/AddonLifecyclePooledArgs.cs

@@ -0,0 +1,107 @@
+using System.Runtime.CompilerServices;
+using System.Threading;
+
+using Dalamud.Game.Addon.Lifecycle.AddonArgTypes;
+
+namespace Dalamud.Game.Addon;
+
+/// <summary>Argument pool for Addon Lifecycle services.</summary>
+[ServiceManager.EarlyLoadedService]
+internal sealed class AddonLifecyclePooledArgs : IServiceType
+{
+    private readonly AddonSetupArgs?[] addonSetupArgPool = new AddonSetupArgs?[64];
+    private readonly AddonFinalizeArgs?[] addonFinalizeArgPool = new AddonFinalizeArgs?[64];
+    private readonly AddonDrawArgs?[] addonDrawArgPool = new AddonDrawArgs?[64];
+    private readonly AddonUpdateArgs?[] addonUpdateArgPool = new AddonUpdateArgs?[64];
+    private readonly AddonRefreshArgs?[] addonRefreshArgPool = new AddonRefreshArgs?[64];
+    private readonly AddonRequestedUpdateArgs?[] addonRequestedUpdateArgPool = new AddonRequestedUpdateArgs?[64];
+    private readonly AddonReceiveEventArgs?[] addonReceiveEventArgPool = new AddonReceiveEventArgs?[64];
+
+    [ServiceManager.ServiceConstructor]
+    private AddonLifecyclePooledArgs()
+    {
+    }
+
+    /// <summary>Rents an instance of an argument.</summary>
+    /// <param name="arg">The rented instance.</param>
+    /// <returns>The returner.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public PooledEntry<AddonSetupArgs> Rent(out AddonSetupArgs arg) => new(out arg, this.addonSetupArgPool);
+
+    /// <summary>Rents an instance of an argument.</summary>
+    /// <param name="arg">The rented instance.</param>
+    /// <returns>The returner.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public PooledEntry<AddonFinalizeArgs> Rent(out AddonFinalizeArgs arg) => new(out arg, this.addonFinalizeArgPool);
+
+    /// <summary>Rents an instance of an argument.</summary>
+    /// <param name="arg">The rented instance.</param>
+    /// <returns>The returner.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public PooledEntry<AddonDrawArgs> Rent(out AddonDrawArgs arg) => new(out arg, this.addonDrawArgPool);
+
+    /// <summary>Rents an instance of an argument.</summary>
+    /// <param name="arg">The rented instance.</param>
+    /// <returns>The returner.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public PooledEntry<AddonUpdateArgs> Rent(out AddonUpdateArgs arg) => new(out arg, this.addonUpdateArgPool);
+
+    /// <summary>Rents an instance of an argument.</summary>
+    /// <param name="arg">The rented instance.</param>
+    /// <returns>The returner.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public PooledEntry<AddonRefreshArgs> Rent(out AddonRefreshArgs arg) => new(out arg, this.addonRefreshArgPool);
+
+    /// <summary>Rents an instance of an argument.</summary>
+    /// <param name="arg">The rented instance.</param>
+    /// <returns>The returner.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public PooledEntry<AddonRequestedUpdateArgs> Rent(out AddonRequestedUpdateArgs arg) =>
+        new(out arg, this.addonRequestedUpdateArgPool);
+
+    /// <summary>Rents an instance of an argument.</summary>
+    /// <param name="arg">The rented instance.</param>
+    /// <returns>The returner.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public PooledEntry<AddonReceiveEventArgs> Rent(out AddonReceiveEventArgs arg) =>
+        new(out arg, this.addonReceiveEventArgPool);
+
+    /// <summary>Returns the object to the pool on dispose.</summary>
+    /// <typeparam name="T">The type.</typeparam>
+    public readonly ref struct PooledEntry<T>
+        where T : AddonArgs, new()
+    {
+        private readonly Span<T> pool;
+        private readonly T obj;
+
+        /// <summary>Initializes a new instance of the <see cref="PooledEntry{T}"/> struct.</summary>
+        /// <param name="arg">An instance of the argument.</param>
+        /// <param name="pool">The pool to rent from and return to.</param>
+        public PooledEntry(out T arg, Span<T> pool)
+        {
+            this.pool = pool;
+            foreach (ref var item in pool)
+            {
+                if (Interlocked.Exchange(ref item, null) is { } v)
+                {
+                    this.obj = arg = v;
+                    return;
+                }
+            }
+
+            this.obj = arg = new();
+        }
+
+        /// <summary>Returns the item to the pool.</summary>
+        public void Dispose()
+        {
+            var tmp = this.obj;
+            foreach (ref var item in this.pool)
+            {
+                if (Interlocked.Exchange(ref item, tmp) is not { } tmp2)
+                    return;
+                tmp = tmp2;
+            }
+        }
+    }
+}

Dalamud/Game/Addon/Events/AddonEventManager.cs

@@ -18,7 +18,7 @@ namespace Dalamud.Game.Addon.Events;
 /// Service provider for addon event management.
 /// </summary>
 [InterfaceVersion("1.0")]
-[ServiceManager.BlockingEarlyLoadedService]
+[ServiceManager.EarlyLoadedService]
 internal unsafe class AddonEventManager : IDisposable, IServiceType
 {
     /// <summary>

Dalamud/Game/Addon/Lifecycle/AddonArgTypes/AddonArgs.cs

@@ -44,10 +44,10 @@ public abstract unsafe class AddonArgs
         get => this.addon;
         set
         {
-            if (this.addon == value)
-                return;
-
             this.addon = value;
+
+            // Note: always clear addonName on updating the addon being pointed.
+            // Same address may point to a different addon.
             this.addonName = null;
         }
     }

Dalamud/Game/Addon/Lifecycle/AddonLifecycle.cs

@@ -1,4 +1,3 @@
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Runtime.CompilerServices;
@@ -19,7 +18,7 @@ namespace Dalamud.Game.Addon.Lifecycle;
 /// This class provides events for in-game addon lifecycles.
 /// </summary>
 [InterfaceVersion("1.0")]
-[ServiceManager.BlockingEarlyLoadedService]
+[ServiceManager.EarlyLoadedService]
 internal unsafe class AddonLifecycle : IDisposable, IServiceType
 {
     private static readonly ModuleLog Log = new("AddonLifecycle");
@@ -27,6 +26,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
     [ServiceManager.ServiceDependency]
     private readonly Framework framework = Service<Framework>.Get();
 
+    [ServiceManager.ServiceDependency]
+    private readonly AddonLifecyclePooledArgs argsPool = Service<AddonLifecyclePooledArgs>.Get();
+
     private readonly nint disallowedReceiveEventAddress;
     
     private readonly AddonLifecycleAddressResolver address;
@@ -38,18 +40,6 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
     private readonly Hook<AddonOnRefreshDelegate> onAddonRefreshHook;
     private readonly CallHook<AddonOnRequestedUpdateDelegate> onAddonRequestedUpdateHook;
 
-    // Note: these can be sourced from ObjectPool of appropriate types instead, but since we don't import that NuGet
-    // package, and these events are always called from the main thread, this is fine.
-#pragma warning disable CS0618 // Type or member is obsolete
-    // TODO: turn constructors of these internal
-    private readonly AddonSetupArgs recyclingSetupArgs = new();
-    private readonly AddonFinalizeArgs recyclingFinalizeArgs = new();
-    private readonly AddonDrawArgs recyclingDrawArgs = new();
-    private readonly AddonUpdateArgs recyclingUpdateArgs = new();
-    private readonly AddonRefreshArgs recyclingRefreshArgs = new();
-    private readonly AddonRequestedUpdateArgs recyclingRequestedUpdateArgs = new();
-#pragma warning restore CS0618 // Type or member is obsolete
-
     [ServiceManager.ServiceConstructor]
     private AddonLifecycle(TargetSigScanner sigScanner)
     {
@@ -253,12 +243,13 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Exception in OnAddonSetup ReceiveEvent Registration.");
         }
 
-        this.recyclingSetupArgs.AddonInternal = (nint)addon;
-        this.recyclingSetupArgs.AtkValueCount = valueCount;
-        this.recyclingSetupArgs.AtkValues = (nint)values;
-        this.InvokeListenersSafely(AddonEvent.PreSetup, this.recyclingSetupArgs);
-        valueCount = this.recyclingSetupArgs.AtkValueCount;
-        values = (AtkValue*)this.recyclingSetupArgs.AtkValues;
+        using var returner = this.argsPool.Rent(out AddonSetupArgs arg);
+        arg.AddonInternal = (nint)addon;
+        arg.AtkValueCount = valueCount;
+        arg.AtkValues = (nint)values;
+        this.InvokeListenersSafely(AddonEvent.PreSetup, arg);
+        valueCount = arg.AtkValueCount;
+        values = (AtkValue*)arg.AtkValues;
 
         try
         {
@@ -269,7 +260,7 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Caught exception when calling original AddonSetup. This may be a bug in the game or another plugin hooking this method.");
         }
 
-        this.InvokeListenersSafely(AddonEvent.PostSetup, this.recyclingSetupArgs);
+        this.InvokeListenersSafely(AddonEvent.PostSetup, arg);
     }
 
     private void OnAddonFinalize(AtkUnitManager* unitManager, AtkUnitBase** atkUnitBase)
@@ -284,8 +275,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Exception in OnAddonFinalize ReceiveEvent Removal.");
         }
 
-        this.recyclingFinalizeArgs.AddonInternal = (nint)atkUnitBase[0];
-        this.InvokeListenersSafely(AddonEvent.PreFinalize, this.recyclingFinalizeArgs);
+        using var returner = this.argsPool.Rent(out AddonFinalizeArgs arg);
+        arg.AddonInternal = (nint)atkUnitBase[0];
+        this.InvokeListenersSafely(AddonEvent.PreFinalize, arg);
 
         try
         {
@@ -299,8 +291,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
 
     private void OnAddonDraw(AtkUnitBase* addon)
     {
-        this.recyclingDrawArgs.AddonInternal = (nint)addon;
-        this.InvokeListenersSafely(AddonEvent.PreDraw, this.recyclingDrawArgs);
+        using var returner = this.argsPool.Rent(out AddonDrawArgs arg);
+        arg.AddonInternal = (nint)addon;
+        this.InvokeListenersSafely(AddonEvent.PreDraw, arg);
 
         try
         {
@@ -311,14 +304,15 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Caught exception when calling original AddonDraw. This may be a bug in the game or another plugin hooking this method.");
         }
 
-        this.InvokeListenersSafely(AddonEvent.PostDraw, this.recyclingDrawArgs);
+        this.InvokeListenersSafely(AddonEvent.PostDraw, arg);
     }
 
     private void OnAddonUpdate(AtkUnitBase* addon, float delta)
     {
-        this.recyclingUpdateArgs.AddonInternal = (nint)addon;
-        this.recyclingUpdateArgs.TimeDeltaInternal = delta;
-        this.InvokeListenersSafely(AddonEvent.PreUpdate, this.recyclingUpdateArgs);
+        using var returner = this.argsPool.Rent(out AddonUpdateArgs arg);
+        arg.AddonInternal = (nint)addon;
+        arg.TimeDeltaInternal = delta;
+        this.InvokeListenersSafely(AddonEvent.PreUpdate, arg);
 
         try
         {
@@ -329,19 +323,20 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Caught exception when calling original AddonUpdate. This may be a bug in the game or another plugin hooking this method.");
         }
 
-        this.InvokeListenersSafely(AddonEvent.PostUpdate, this.recyclingUpdateArgs);
+        this.InvokeListenersSafely(AddonEvent.PostUpdate, arg);
     }
 
     private byte OnAddonRefresh(AtkUnitManager* atkUnitManager, AtkUnitBase* addon, uint valueCount, AtkValue* values)
     {
         byte result = 0;
 
-        this.recyclingRefreshArgs.AddonInternal = (nint)addon;
-        this.recyclingRefreshArgs.AtkValueCount = valueCount;
-        this.recyclingRefreshArgs.AtkValues = (nint)values;
-        this.InvokeListenersSafely(AddonEvent.PreRefresh, this.recyclingRefreshArgs);
-        valueCount = this.recyclingRefreshArgs.AtkValueCount;
-        values = (AtkValue*)this.recyclingRefreshArgs.AtkValues;
+        using var returner = this.argsPool.Rent(out AddonRefreshArgs arg);
+        arg.AddonInternal = (nint)addon;
+        arg.AtkValueCount = valueCount;
+        arg.AtkValues = (nint)values;
+        this.InvokeListenersSafely(AddonEvent.PreRefresh, arg);
+        valueCount = arg.AtkValueCount;
+        values = (AtkValue*)arg.AtkValues;
 
         try
         {
@@ -352,18 +347,19 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Caught exception when calling original AddonRefresh. This may be a bug in the game or another plugin hooking this method.");
         }
 
-        this.InvokeListenersSafely(AddonEvent.PostRefresh, this.recyclingRefreshArgs);
+        this.InvokeListenersSafely(AddonEvent.PostRefresh, arg);
         return result;
     }
 
     private void OnRequestedUpdate(AtkUnitBase* addon, NumberArrayData** numberArrayData, StringArrayData** stringArrayData)
     {
-        this.recyclingRequestedUpdateArgs.AddonInternal = (nint)addon;
-        this.recyclingRequestedUpdateArgs.NumberArrayData = (nint)numberArrayData;
-        this.recyclingRequestedUpdateArgs.StringArrayData = (nint)stringArrayData;
-        this.InvokeListenersSafely(AddonEvent.PreRequestedUpdate, this.recyclingRequestedUpdateArgs);
-        numberArrayData = (NumberArrayData**)this.recyclingRequestedUpdateArgs.NumberArrayData;
-        stringArrayData = (StringArrayData**)this.recyclingRequestedUpdateArgs.StringArrayData;
+        using var returner = this.argsPool.Rent(out AddonRequestedUpdateArgs arg);
+        arg.AddonInternal = (nint)addon;
+        arg.NumberArrayData = (nint)numberArrayData;
+        arg.StringArrayData = (nint)stringArrayData;
+        this.InvokeListenersSafely(AddonEvent.PreRequestedUpdate, arg);
+        numberArrayData = (NumberArrayData**)arg.NumberArrayData;
+        stringArrayData = (StringArrayData**)arg.StringArrayData;
 
         try
         {
@@ -374,7 +370,7 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Caught exception when calling original AddonRequestedUpdate. This may be a bug in the game or another plugin hooking this method.");
         }
 
-        this.InvokeListenersSafely(AddonEvent.PostRequestedUpdate, this.recyclingRequestedUpdateArgs);
+        this.InvokeListenersSafely(AddonEvent.PostRequestedUpdate, arg);
     }
 }
 

Dalamud/Game/Addon/Lifecycle/AddonLifecycleReceiveEventListener.cs

@@ -16,12 +16,8 @@ internal unsafe class AddonLifecycleReceiveEventListener : IDisposable
 {
     private static readonly ModuleLog Log = new("AddonLifecycle");
 
-    // Note: these can be sourced from ObjectPool of appropriate types instead, but since we don't import that NuGet
-    // package, and these events are always called from the main thread, this is fine.
-#pragma warning disable CS0618 // Type or member is obsolete
-    // TODO: turn constructors of these internal
-    private readonly AddonReceiveEventArgs recyclingReceiveEventArgs = new();
-#pragma warning restore CS0618 // Type or member is obsolete
+    [ServiceManager.ServiceDependency]
+    private readonly AddonLifecyclePooledArgs argsPool = Service<AddonLifecyclePooledArgs>.Get();
 
     /// <summary>
     /// Initializes a new instance of the <see cref="AddonLifecycleReceiveEventListener"/> class.
@@ -82,16 +78,17 @@ internal unsafe class AddonLifecycleReceiveEventListener : IDisposable
             return;
         }
 
-        this.recyclingReceiveEventArgs.AddonInternal = (nint)addon;
-        this.recyclingReceiveEventArgs.AtkEventType = (byte)eventType;
-        this.recyclingReceiveEventArgs.EventParam = eventParam;
-        this.recyclingReceiveEventArgs.AtkEvent = (IntPtr)atkEvent;
-        this.recyclingReceiveEventArgs.Data = data;
-        this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PreReceiveEvent, this.recyclingReceiveEventArgs);
-        eventType = (AtkEventType)this.recyclingReceiveEventArgs.AtkEventType;
-        eventParam = this.recyclingReceiveEventArgs.EventParam;
-        atkEvent = (AtkEvent*)this.recyclingReceiveEventArgs.AtkEvent;
-        data = this.recyclingReceiveEventArgs.Data;
+        using var returner = this.argsPool.Rent(out AddonReceiveEventArgs arg);
+        arg.AddonInternal = (nint)addon;
+        arg.AtkEventType = (byte)eventType;
+        arg.EventParam = eventParam;
+        arg.AtkEvent = (IntPtr)atkEvent;
+        arg.Data = data;
+        this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PreReceiveEvent, arg);
+        eventType = (AtkEventType)arg.AtkEventType;
+        eventParam = arg.EventParam;
+        atkEvent = (AtkEvent*)arg.AtkEvent;
+        data = arg.Data;
         
         try
         {
@@ -102,6 +99,6 @@ internal unsafe class AddonLifecycleReceiveEventListener : IDisposable
             Log.Error(e, "Caught exception when calling original AddonReceiveEvent. This may be a bug in the game or another plugin hooking this method.");
         }
 
-        this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PostReceiveEvent, this.recyclingReceiveEventArgs);
+        this.AddonLifecycle.InvokeListenersSafely(AddonEvent.PostReceiveEvent, arg);
     }
 }