fix event-invocation on AOT targets (#3158)

investigate and fix #3157

Author: mgravell

src/StackExchange.Redis/Delegates.cs

@@ -1,4 +1,8 @@
-#if NET8_0_OR_GREATER
+#if NET9_0_OR_GREATER
+// .NET 9 added Delegate.EnumerateInvocationList, which is exactly the allocation-free
+// enumerator we want, is runtime-agnostic, and works on NativeAOT; prefer it
+#define BCL_INVOCATION_LIST
+#elif NET8_0_OR_GREATER
 #define UNSAFE_ACCESSOR // retain ability to disable easily
 #endif
 
@@ -7,7 +11,7 @@
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
 
-#if !UNSAFE_ACCESSOR
+#if !UNSAFE_ACCESSOR && !BCL_INVOCATION_LIST
 using System.Reflection;
 using System.Reflection.Emit;
 #endif
@@ -25,7 +29,7 @@ internal static class Delegates
     /// <typeparam name="T">The type of delegate being enumerated.</typeparam>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static DelegateEnumerator<T> GetEnumerator<T>(this T? handler) where T : MulticastDelegate
-        => handler is null ? default : new(handler);
+        => new(handler);
 
     /// <summary>
     /// Iterate over the individual elements of a multicast delegate (without allocation).
@@ -41,8 +45,15 @@ public static DelegateEnumerable<T> AsEnumerable<T>(this T? handler) where T : M
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool IsSingle(this MulticastDelegate handler)
     {
-#if UNSAFE_ACCESSOR
-        return s_getArr(handler) is null;
+#if BCL_INVOCATION_LIST
+        var iterator = Delegate.EnumerateInvocationList(handler);
+        return iterator.MoveNext() && !iterator.MoveNext();
+#elif UNSAFE_ACCESSOR
+        if (s_isAvailable)
+        {
+            return s_getArr(handler) is null;
+        }
+        return handler.GetInvocationList().Length == 1;
 #else
         if (s_isAvailable)
         {
@@ -63,7 +74,11 @@ public static bool IsSingle(this MulticastDelegate handler)
     /// </summary>
     public static bool IsSupported => s_isAvailable;
 
-#if UNSAFE_ACCESSOR
+#if BCL_INVOCATION_LIST
+#pragma warning disable SA1303
+    private const bool s_isAvailable = true;
+#pragma warning restore SA1303
+#elif UNSAFE_ACCESSOR
 #pragma warning disable SA1300
     [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "_invocationList")]
     private static extern ref readonly object? s_getArr(MulticastDelegate handler);
@@ -73,10 +88,34 @@ public static bool IsSingle(this MulticastDelegate handler)
 
     // ReSharper disable once InconsistentNaming
 #pragma warning disable SA1303
-    private const bool s_isAvailable = true;
+    private static readonly bool s_isAvailable = IsAvailable();
 #pragma warning restore SA1303
 
 #pragma warning restore SA1300
+
+    private static bool IsAvailable()
+    {
+        // these fields are an implementation detail of the CoreCLR/.NET Framework delegate layout;
+        // other runtimes differ (NativeAOT keeps the invocation list on Delegate, in a different
+        // shape), in which case the accessors above throw MissingFieldException on first use - so:
+        // validate before we trust them, and fall back to GetInvocationList() when unsure (see #3157)
+        try
+        {
+            Action probe = Probe;
+            if (s_getArr(probe) is not null) return false; // expect: single-target => no list
+
+            probe += Probe; // (combine does not de-duplicate, so this is genuinely two targets)
+            return s_getArr(probe) is object[] arr && arr.Length >= 2 // (the array can have spare capacity)
+                && (int)s_getCount(probe) == 2
+                && arr[0] is Action && arr[1] is Action;
+        }
+        catch
+        {
+            return false;
+        }
+
+        static void Probe() { }
+    }
 #else
 #pragma warning disable SA1300
     private static readonly Func<MulticastDelegate, object?>? s_getArr;
@@ -153,8 +192,7 @@ private static Func<MulticastDelegate, T> GetViaReflection<T>(FieldInfo field)
         /// Iterate over the individual elements of a multicast delegate (without allocation).
         /// </summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public DelegateEnumerator<T> GetEnumerator()
-            => _handler is null ? default : new DelegateEnumerator<T>(_handler);
+        public DelegateEnumerator<T> GetEnumerator() => new(_handler);
         IEnumerator<T> IEnumerable<T>.GetEnumerator() => GetEnumerator();
         IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
     }
@@ -165,21 +203,33 @@ public DelegateEnumerator<T> GetEnumerator()
     /// <typeparam name="T">The type of delegate being enumerated.</typeparam>
     public struct DelegateEnumerator<T> : IEnumerator<T> where T : MulticastDelegate
     {
-        private readonly T _handler;
+        private readonly T? _handler;
+#if BCL_INVOCATION_LIST
+        private Delegate.InvocationListEnumerator<T> _iterator;
+#else
         private readonly object[]? _arr;
         private readonly int _count;
         private int _index;
         private T? _current;
-        internal DelegateEnumerator(T handler)
+#endif
+
+        internal DelegateEnumerator(T? handler)
         {
-            // Debug.Assert(handler is not null);
             _handler = handler;
-#if UNSAFE_ACCESSOR
-            _arr = s_getArr(handler) as object[];
-            _count = _arr is null ? 1 : (int)s_getCount(handler);
+#if BCL_INVOCATION_LIST
+            _iterator = Delegate.EnumerateInvocationList(handler);
 #else
-            if (s_isAvailable)
+            if (handler is null)
+            {
+                _arr = null;
+                _count = 0;
+            }
+            else if (s_isAvailable)
             {
+#if UNSAFE_ACCESSOR
+                _arr = s_getArr(handler) as object[];
+                _count = _arr is null ? 1 : (int)s_getCount(handler);
+#else
                 if (s_delegates is null)
                 {
                     _arr = s_getArr!(handler) as object[];
@@ -191,16 +241,18 @@ internal DelegateEnumerator(T handler)
                     _arr = s_delegates(handler);
                     _count = _arr?.Length ?? 1;
                 }
+#endif
             }
             else
             {
                 // ReSharper disable once CoVariantArrayConversion
                 _arr = handler.GetInvocationList();
                 _count = _arr.Length;
             }
-#endif
+
             _current = null;
             _index = -1;
+#endif
         }
 
         /// <summary>
@@ -209,7 +261,11 @@ internal DelegateEnumerator(T handler)
         public T Current
         {
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
+#if BCL_INVOCATION_LIST
+            get => _iterator.Current;
+#else
             get => _current!;
+#endif
         }
 
         object? IEnumerator.Current => Current;
@@ -222,24 +278,32 @@ void IDisposable.Dispose() { }
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public bool MoveNext()
         {
+#if BCL_INVOCATION_LIST
+            return _iterator.MoveNext();
+#else
             var next = _index + 1;
             if (next >= _count)
             {
                 _current = null;
                 return false;
             }
-            _current = _arr is null ? _handler : (T)_arr[next];
+            _current = _arr is null ? _handler! : (T)_arr[next];
             _index = next;
             return true;
+#endif
         }
 
         /// <summary>
         /// Reset the enumerator, allowing the sequence to be repeated.
         /// </summary>
         public void Reset()
         {
+#if BCL_INVOCATION_LIST
+            _iterator = Delegate.EnumerateInvocationList(_handler);
+#else
             _current = null;
             _index = -1;
+#endif
         }
     }
 }

tests/StackExchange.Redis.Tests/DelegateTests.cs

@@ -51,4 +51,69 @@ public void Foo(int count)
             Assert.Equal(i, captured[i]);
         }
     }
+
+    [Theory]
+    [InlineData(1)]
+    [InlineData(2)]
+    [InlineData(25)]
+    public void MatchesGetInvocationList(int count)
+    {
+        Action? action = null;
+        for (int i = 0; i < count; i++)
+        {
+            action += Noop;
+        }
+
+        Assert.NotNull(action);
+        var expected = action.GetInvocationList();
+        Assert.Equal(count, expected.Length);
+        Assert.Equal(count == 1, action.IsSingle());
+
+        int index = 0;
+        foreach (var inner in action.AsEnumerable())
+        {
+            Assert.Same(expected[index++], inner);
+        }
+        Assert.Equal(count, index);
+
+        // and again, to check that removal keeps things consistent
+        action -= Noop;
+        if (count == 1)
+        {
+            Assert.Null(action);
+            return;
+        }
+
+        Assert.NotNull(action);
+        expected = action.GetInvocationList();
+        Assert.Equal(count - 1, expected.Length);
+        Assert.Equal(count == 2, action.IsSingle());
+        index = 0;
+        foreach (var inner in action.AsEnumerable())
+        {
+            Assert.Same(expected[index++], inner);
+        }
+        Assert.Equal(count - 1, index);
+
+        static void Noop() { }
+    }
+
+    [Fact]
+    public void ResetRepeatsSequence()
+    {
+        Action? action = Noop;
+        action += Noop;
+
+        var iterator = action.GetEnumerator();
+        Assert.True(iterator.MoveNext());
+        Assert.True(iterator.MoveNext());
+        Assert.False(iterator.MoveNext());
+
+        iterator.Reset();
+        int count = 0;
+        while (iterator.MoveNext()) count++;
+        Assert.Equal(2, count);
+
+        static void Noop() { }
+    }
 }

toys/AotRig/AotRig.csproj

@@ -0,0 +1,19 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <!-- Minimal native-AOT rig: connects, does a few things, and exercises the event
+       machinery (which is where AOT-hostile tricks tend to surface); see #3157. -->
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <!-- net8.0 as well as net10.0: the library uses a different invocation-list strategy on each
+         (UnsafeAccessor + validation vs. the BCL's Delegate.EnumerateInvocationList) -->
+    <TargetFrameworks>net8.0;net10.0</TargetFrameworks>
+    <Nullable>enable</Nullable>
+    <PublishAot>true</PublishAot>
+    <InvariantGlobalization>true</InvariantGlobalization>
+    <StackTraceSupport>true</StackTraceSupport>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\src\StackExchange.Redis\StackExchange.Redis.csproj" />
+  </ItemGroup>
+</Project>