diff --git a/src/StackExchange.Redis/Delegates.cs b/src/StackExchange.Redis/Delegates.cs index 66e97adec..956d2171a 100644 --- a/src/StackExchange.Redis/Delegates.cs +++ b/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 /// The type of delegate being enumerated. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static DelegateEnumerator GetEnumerator(this T? handler) where T : MulticastDelegate - => handler is null ? default : new(handler); + => new(handler); /// /// Iterate over the individual elements of a multicast delegate (without allocation). @@ -41,8 +45,15 @@ public static DelegateEnumerable AsEnumerable(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) /// 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? s_getArr; @@ -153,8 +192,7 @@ private static Func GetViaReflection(FieldInfo field) /// Iterate over the individual elements of a multicast delegate (without allocation). /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public DelegateEnumerator GetEnumerator() - => _handler is null ? default : new DelegateEnumerator(_handler); + public DelegateEnumerator GetEnumerator() => new(_handler); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } @@ -165,21 +203,33 @@ public DelegateEnumerator GetEnumerator() /// The type of delegate being enumerated. public struct DelegateEnumerator : IEnumerator where T : MulticastDelegate { - private readonly T _handler; + private readonly T? _handler; +#if BCL_INVOCATION_LIST + private Delegate.InvocationListEnumerator _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,6 +241,7 @@ internal DelegateEnumerator(T handler) _arr = s_delegates(handler); _count = _arr?.Length ?? 1; } +#endif } else { @@ -198,9 +249,10 @@ internal DelegateEnumerator(T handler) _arr = handler.GetInvocationList(); _count = _arr.Length; } -#endif + _current = null; _index = -1; +#endif } /// @@ -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,15 +278,19 @@ 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 } /// @@ -238,8 +298,12 @@ public bool MoveNext() /// public void Reset() { +#if BCL_INVOCATION_LIST + _iterator = Delegate.EnumerateInvocationList(_handler); +#else _current = null; _index = -1; +#endif } } } diff --git a/tests/StackExchange.Redis.Tests/DelegateTests.cs b/tests/StackExchange.Redis.Tests/DelegateTests.cs index c36dcf79a..3832022a9 100644 --- a/tests/StackExchange.Redis.Tests/DelegateTests.cs +++ b/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() { } + } } diff --git a/toys/AotRig/AotRig.csproj b/toys/AotRig/AotRig.csproj new file mode 100644 index 000000000..e8312f4fe --- /dev/null +++ b/toys/AotRig/AotRig.csproj @@ -0,0 +1,19 @@ + + + + + Exe + + net8.0;net10.0 + enable + true + true + true + + + + + + diff --git a/toys/AotRig/Program.cs b/toys/AotRig/Program.cs new file mode 100644 index 000000000..25f2b43c5 --- /dev/null +++ b/toys/AotRig/Program.cs @@ -0,0 +1,198 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using StackExchange.Redis; + +namespace AotRig; + +/// +/// Minimal rig that connects and does a few things; intended to be published with +/// PublishAot=true so that AOT-hostile code paths (in particular the manually +/// unrolled event invocation, see issue #3157) fail loudly rather than silently. +/// +internal static class Program +{ + private static int s_failures; + + private static async Task Main(string[] args) + { + var alive = args.Length > 0 ? args[0] : "127.0.0.1:6379"; + var dead = args.Length > 1 ? args[1] : "127.0.0.1:6390"; + + Console.WriteLine($"runtime: {System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription}"); + Console.WriteLine($"dynamic code: {System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported}"); + Console.WriteLine(); + + await Run("connection-failed events", () => ConnectionFailedEvents(dead)).ConfigureAwait(false); + await Run("basic operations", () => BasicOperations(alive)).ConfigureAwait(false); + await Run("pub/sub handlers", () => PubSub(alive)).ConfigureAwait(false); + await Run("connection events (live server)", () => LiveServerEvents(alive)).ConfigureAwait(false); + + Console.WriteLine(); + Console.WriteLine(s_failures == 0 ? "ALL PASS" : $"{s_failures} FAILED"); + return s_failures; + } + + private static async Task Run(string name, Func action) + { + Console.Write($"{name}... "); + try + { + await action().ConfigureAwait(false); + Console.WriteLine("pass"); + } + catch (Exception ex) + { + Interlocked.Increment(ref s_failures); + Console.WriteLine($"FAIL: {ex.GetType().Name}: {ex.Message}"); + Console.WriteLine(ex.StackTrace); + } + } + + // #3157: a failed connection raises ConnectionFailed on the thread-pool; the multiplexer + // unrolls the multicast delegate by hand, which is where AOT bites. + private static async Task ConnectionFailedEvents(string dead) + { + var options = new ConfigurationOptions + { + AbortOnConnectFail = false, + ConnectTimeout = 1000, + ConnectRetry = 1, + AllowAdmin = true, + }; + options.EndPoints.Add(dead); + + int failed = 0, internalError = 0; + var signal = new SemaphoreSlim(0); + await using var muxer = await ConnectionMultiplexer.ConnectAsync(options).ConfigureAwait(false); + muxer.ConnectionFailed += (_, _) => Interlocked.Increment(ref failed); + muxer.ConnectionFailed += (_, _) => + { + // deliberately a second handler, so the multicast path is used too + signal.Release(); + }; + muxer.InternalError += (_, _) => Interlocked.Increment(ref internalError); + + // the initial (failed) connect happens before we can attach handlers, so keep poking it: + // each attempt is another connect failure, and therefore another event + var db = muxer.GetDatabase(); + var deadline = Environment.TickCount64 + 30_000; + while (!await signal.WaitAsync(250).ConfigureAwait(false)) + { + if (Environment.TickCount64 > deadline) + { + throw new TimeoutException($"no ConnectionFailed event within timeout (single-handler count: {Volatile.Read(ref failed)}, internal errors: {Volatile.Read(ref internalError)})"); + } + + try + { + await db.PingAsync().ConfigureAwait(false); + } + catch (RedisConnectionException) + { + // expected + } + } + + Console.Write($"[failed:{Volatile.Read(ref failed)} internal:{Volatile.Read(ref internalError)}] "); + } + + private static async Task BasicOperations(string alive) + { + var options = ConfigurationOptions.Parse(alive); + options.AllowAdmin = true; + await using var muxer = await ConnectionMultiplexer.ConnectAsync(options).ConfigureAwait(false); + var db = muxer.GetDatabase(); + + RedisKey key = "aot-rig:value"; + await db.KeyDeleteAsync(key).ConfigureAwait(false); + await db.StringSetAsync(key, "hello").ConfigureAwait(false); + var val = await db.StringGetAsync(key).ConfigureAwait(false); + if (val != "hello") throw new InvalidOperationException($"unexpected value: {val}"); + + RedisKey counter = "aot-rig:counter"; + await db.KeyDeleteAsync(counter).ConfigureAwait(false); + if (await db.StringIncrementAsync(counter, 4).ConfigureAwait(false) != 4) throw new InvalidOperationException("INCRBY"); + + RedisKey hash = "aot-rig:hash"; + await db.KeyDeleteAsync(hash).ConfigureAwait(false); + await db.HashSetAsync(hash, [new HashEntry("a", 1), new HashEntry("b", 2)]).ConfigureAwait(false); + if ((await db.HashGetAllAsync(hash).ConfigureAwait(false)).Length != 2) throw new InvalidOperationException("HGETALL"); + + var latency = await db.PingAsync().ConfigureAwait(false); + Console.Write($"[ping:{latency.TotalMilliseconds:0.##}ms] "); + + var server = muxer.GetServer(muxer.GetEndPoints()[0]); + _ = server.Version; + await db.KeyDeleteAsync([key, counter, hash]).ConfigureAwait(false); + } + + private static async Task PubSub(string alive) + { + await using var muxer = await ConnectionMultiplexer.ConnectAsync(alive).ConfigureAwait(false); + var sub = muxer.GetSubscriber(); + RedisChannel channel = RedisChannel.Literal("aot-rig:channel"); + + int hits = 0; + var signal = new SemaphoreSlim(0); + // two handlers on the same channel: exercises the multicast unroll in the pub/sub path + await sub.SubscribeAsync(channel, (_, _) => Interlocked.Increment(ref hits)).ConfigureAwait(false); + await sub.SubscribeAsync(channel, (_, _) => signal.Release()).ConfigureAwait(false); + + await sub.PublishAsync(channel, "ping").ConfigureAwait(false); + if (!await signal.WaitAsync(10_000).ConfigureAwait(false)) + { + throw new TimeoutException($"no message within timeout (other handler: {Volatile.Read(ref hits)})"); + } + + await sub.UnsubscribeAllAsync().ConfigureAwait(false); + Console.Write($"[hits:{Volatile.Read(ref hits)}] "); + } + + // ConnectionRestored / ConnectionFailed against a real server, via a forced reconnect + private static async Task LiveServerEvents(string alive) + { + const string ClientName = "aot-rig"; + var options = ConfigurationOptions.Parse(alive); + options.AllowAdmin = true; + options.AbortOnConnectFail = false; + options.ClientName = ClientName; + await using var muxer = await ConnectionMultiplexer.ConnectAsync(options).ConfigureAwait(false); + + int restored = 0, failed = 0; + var signal = new SemaphoreSlim(0); + muxer.ConnectionRestored += (_, _) => Interlocked.Increment(ref restored); + muxer.ConnectionRestored += (_, _) => signal.Release(); + muxer.ConnectionFailed += (_, _) => Interlocked.Increment(ref failed); + muxer.ErrorMessage += (_, _) => { }; + + var db = muxer.GetDatabase(); + await db.PingAsync().ConfigureAwait(false); + + // kill our own connections server-side; the multiplexer should notice and re-establish, + // raising ConnectionFailed/ConnectionRestored through the machinery we care about + var server = muxer.GetServer(muxer.GetEndPoints()[0]); + foreach (var client in await server.ClientListAsync().ConfigureAwait(false)) + { + if (client.Name == ClientName) + { + try + { + await server.ClientKillAsync(id: client.Id, skipMe: false).ConfigureAwait(false); + } + catch (RedisConnectionException) + { + // expected when we kill the connection issuing the command + } + } + } + + if (!await signal.WaitAsync(20_000).ConfigureAwait(false)) + { + throw new TimeoutException($"no ConnectionRestored within timeout (failed: {Volatile.Read(ref failed)})"); + } + + await db.PingAsync().ConfigureAwait(false); + Console.Write($"[failed:{Volatile.Read(ref failed)} restored:{Volatile.Read(ref restored)}] "); + } +}