From 185affbc175fa7b8be18b0c146c7ef38f9c50287 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:12:56 +0000 Subject: [PATCH 01/19] Track per-partition last-access tick so NoopLimiter is evicted by Heartbeat Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17 Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../DefaultPartitionedRateLimiter.cs | 67 +++++++++++----- .../tests/PartitionedRateLimiterTests.cs | 76 +++++++++++++++++++ 2 files changed, 126 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index b2966bf85c05ae..887aee63440b5e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -16,14 +16,14 @@ internal sealed class DefaultPartitionedRateLimiter : Partition private static readonly TimeSpan s_idleTimeLimit = TimeSpan.FromSeconds(10); // TODO: Look at ConcurrentDictionary to try and avoid a global lock - private readonly Dictionary> _limiters; + private readonly Dictionary> _limiters; private bool _disposed; private readonly TaskCompletionSource _disposeComplete = new(TaskCreationOptions.RunContinuationsAsynchronously); // Used by the Timer to call TryRelenish on ReplenishingRateLimiters // We use a separate list to avoid running TryReplenish (which might be user code) inside our lock // And we cache the list to amortize the allocation cost to as close to 0 as we can get - private readonly List>> _cachedLimiters = new(); + private readonly List>> _cachedLimiters = new(); private bool _cacheInvalid; private readonly List _limitersToDispose = new(); private readonly TimerAwaitable _timer; @@ -42,7 +42,7 @@ public DefaultPartitionedRateLimiter(Func> p private DefaultPartitionedRateLimiter(Func> partitioner, IEqualityComparer? equalityComparer, TimeSpan timerInterval) { - _limiters = new Dictionary>(equalityComparer); + _limiters = new Dictionary>(equalityComparer); _partitioner = partitioner; _timer = new TimerAwaitable(timerInterval, timerInterval); @@ -87,20 +87,31 @@ protected override ValueTask AcquireAsyncCore(TResource resource private RateLimiter GetRateLimiter(TResource resource) { RateLimitPartition partition = _partitioner(resource); - Lazy? limiter; + Lazy? entry; lock (Lock) { ThrowIfDisposed(); - if (!_limiters.TryGetValue(partition.PartitionKey, out limiter)) + if (!_limiters.TryGetValue(partition.PartitionKey, out entry)) { // Using Lazy avoids calling user code (partition.Factory) inside the lock - limiter = new Lazy(() => partition.Factory(partition.PartitionKey)); - _limiters.Add(partition.PartitionKey, limiter); + entry = new Lazy(() => new LimiterEntry(partition.Factory(partition.PartitionKey))); + _limiters.Add(partition.PartitionKey, entry); // Cache is invalid now _cacheInvalid = true; } + else if (entry.IsValueCreated) + { + // Refresh the last-access timestamp under the lock so the Heartbeat won't observe + // a stale value and concurrently evict a limiter that's actively being used. + entry.Value.LastAccessTimestamp = Stopwatch.GetTimestamp(); + } } - return limiter.Value; + + LimiterEntry limiterEntry = entry.Value; + // Refresh the last-access timestamp so that the Heartbeat can evict limiters whose + // IdleDuration is always null (e.g. NoopLimiter) once they stop being accessed. + limiterEntry.LastAccessTimestamp = Stopwatch.GetTimestamp(); + return limiterEntry.Limiter; } protected override void Dispose(bool disposing) @@ -125,11 +136,11 @@ protected override void Dispose(bool disposing) // Safe to access _limiters outside the lock // The timer is no longer running and _disposed is set so anyone trying to access fields will be checking that first - foreach (KeyValuePair> limiter in _limiters) + foreach (KeyValuePair> limiter in _limiters) { try { - limiter.Value.Value.Dispose(); + limiter.Value.Value.Limiter.Dispose(); } catch (Exception ex) { @@ -160,11 +171,11 @@ protected override async ValueTask DisposeAsyncCore() } List? exceptions = null; - foreach (KeyValuePair> limiter in _limiters) + foreach (KeyValuePair> limiter in _limiters) { try { - await limiter.Value.Value.DisposeAsync().ConfigureAwait(false); + await limiter.Value.Value.Limiter.DisposeAsync().ConfigureAwait(false); } catch (Exception ex) { @@ -225,18 +236,25 @@ private async Task Heartbeat() List? aggregateExceptions = null; // cachedLimiters is safe to use outside the lock because it is only updated by the Timer - foreach (KeyValuePair> rateLimiter in _cachedLimiters) + foreach (KeyValuePair> rateLimiter in _cachedLimiters) { if (!rateLimiter.Value.IsValueCreated) { continue; } - if (rateLimiter.Value.Value.IdleDuration is TimeSpan idleDuration && idleDuration > s_idleTimeLimit) + LimiterEntry limiterEntry = rateLimiter.Value.Value; + // Fall back to our internally tracked last-access timestamp when the limiter + // does not report an idle duration (e.g. NoopLimiter always returns null). + // This ensures idle partitions are eventually evicted regardless of the limiter implementation. + TimeSpan idleDuration = limiterEntry.Limiter.IdleDuration + ?? RateLimiterHelper.GetElapsedTime(limiterEntry.LastAccessTimestamp).GetValueOrDefault(); + if (idleDuration > s_idleTimeLimit) { lock (Lock) { // Check time again under lock to make sure no one calls Acquire or WaitAsync after checking the time and removing the limiter - idleDuration = rateLimiter.Value.Value.IdleDuration ?? TimeSpan.Zero; + idleDuration = limiterEntry.Limiter.IdleDuration + ?? RateLimiterHelper.GetElapsedTime(limiterEntry.LastAccessTimestamp).GetValueOrDefault(); if (idleDuration > s_idleTimeLimit) { // Remove limiter from the lookup table and mark cache as invalid @@ -246,12 +264,12 @@ private async Task Heartbeat() _limiters.Remove(rateLimiter.Key); // We don't want to dispose inside the lock so we need to defer it - _limitersToDispose.Add(rateLimiter.Value.Value); + _limitersToDispose.Add(limiterEntry.Limiter); } } } // We know the limiter can be replenished so let's attempt to replenish tokens - else if (rateLimiter.Value.Value is ReplenishingRateLimiter replenishingRateLimiter) + else if (limiterEntry.Limiter is ReplenishingRateLimiter replenishingRateLimiter) { try { @@ -284,5 +302,20 @@ private async Task Heartbeat() throw new AggregateException(aggregateExceptions); } } + + // Wraps a RateLimiter with a timestamp of when it was last accessed by the partitioned limiter. + // The timestamp is used by the Heartbeat to evict limiters whose IdleDuration is always null + // (e.g. NoopLimiter), which would otherwise never be cleaned up. + private sealed class LimiterEntry + { + public LimiterEntry(RateLimiter limiter) + { + Limiter = limiter; + LastAccessTimestamp = Stopwatch.GetTimestamp(); + } + + public RateLimiter Limiter { get; } + public long LastAccessTimestamp; + } } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index ff6e7c4264941b..9b7cb450046ab1 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -529,6 +529,82 @@ public async Task IdleLimiterIsCleanedUp() Assert.Equal(2, factoryCallCount); } + [Fact] + public async Task IdleLimiterWithNullIdleDurationIsCleanedUp() + { + // Verifies that a limiter that always reports IdleDuration == null (e.g. NoopLimiter) + // is still evicted by the heartbeat once it has not been accessed for the idle window. + // Regression coverage for the NoopLimiter memory leak scenario. + var factoryCallCount = 0; + CustomizableLimiter innerLimiter = null; + using var limiter = Utils.CreatePartitionedLimiterWithoutTimer(resource => + { + return RateLimitPartition.Get(1, _ => + { + factoryCallCount++; + innerLimiter = new CustomizableLimiter + { + IdleDurationImpl = () => null + }; + return innerLimiter; + }); + }); + + var lease = limiter.AttemptAcquire(""); + Assert.True(lease.IsAcquired); + Assert.Equal(1, factoryCallCount); + + var disposeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + innerLimiter.DisposeAsyncCoreImpl = () => + { + disposeTcs.SetResult(null); + return default; + }; + + // Backdate the internally-tracked last-access timestamp so the heartbeat treats the + // partition as idle even though the inner limiter reports IdleDuration == null. + BackdateLastAccessTimestamp(limiter, key: 1); + + await Utils.RunTimerFunc(limiter); + + // Limiter is disposed when timer runs and sees that the internal idle tracking is greater than the idle limit + await disposeTcs.Task; + innerLimiter.DisposeAsyncCoreImpl = () => default; + + // Acquire will call limiter factory again as the limiter was disposed and removed + lease = limiter.AttemptAcquire(""); + Assert.True(lease.IsAcquired); + Assert.Equal(2, factoryCallCount); + } + + // Reaches into the internal _limiters dictionary of DefaultPartitionedRateLimiter and + // backdates the LastAccessTimestamp on the LimiterEntry for the given key so the test + // doesn't have to wait the real idle window before the heartbeat evicts the limiter. + private static void BackdateLastAccessTimestamp(PartitionedRateLimiter limiter, int key) + { + var limiterTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2, System.Threading.RateLimiting"); + var limiterType = limiter.GetType(); + Assert.Equal(limiterTypeDef, limiterType.GetGenericTypeDefinition()); + + var limitersField = limiterType.GetField("_limiters", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + Assert.NotNull(limitersField); + var limitersDict = (System.Collections.IDictionary)limitersField.GetValue(limiter); + Assert.NotNull(limitersDict); + + var lazyEntry = limitersDict[key]; + Assert.NotNull(lazyEntry); + + // Force creation of the Lazy.Value + var lazyType = lazyEntry.GetType(); + var entry = lazyType.GetProperty("Value").GetValue(lazyEntry); + Assert.NotNull(entry); + + // LastAccessTimestamp is a public mutable field on the internal LimiterEntry type. + var lastAccessField = entry.GetType().GetField("LastAccessTimestamp"); + Assert.NotNull(lastAccessField); + lastAccessField.SetValue(entry, 0L); + } + [Fact] public async Task AllIdleLimitersCleanedUp_DisposeThrows() { From 4b31962c41fdb015b8f625c69a3642c3a36eae1a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:15:19 +0000 Subject: [PATCH 02/19] Address review: remove redundant timestamp write, use Interlocked for atomic 64-bit access Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17 Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../DefaultPartitionedRateLimiter.cs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index 887aee63440b5e..5caff3570a3dcc 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -93,7 +93,8 @@ private RateLimiter GetRateLimiter(TResource resource) ThrowIfDisposed(); if (!_limiters.TryGetValue(partition.PartitionKey, out entry)) { - // Using Lazy avoids calling user code (partition.Factory) inside the lock + // Using Lazy avoids calling user code (partition.Factory) inside the lock. + // The LimiterEntry constructor initializes LastAccessTimestamp when the factory runs. entry = new Lazy(() => new LimiterEntry(partition.Factory(partition.PartitionKey))); _limiters.Add(partition.PartitionKey, entry); // Cache is invalid now @@ -101,17 +102,15 @@ private RateLimiter GetRateLimiter(TResource resource) } else if (entry.IsValueCreated) { - // Refresh the last-access timestamp under the lock so the Heartbeat won't observe - // a stale value and concurrently evict a limiter that's actively being used. - entry.Value.LastAccessTimestamp = Stopwatch.GetTimestamp(); + // Refresh the last-access timestamp under the lock so the Heartbeat won't + // observe a stale value and concurrently evict a limiter that's actively being used. + // Use Interlocked.Exchange so the write is atomic on 32-bit platforms where the + // outside-lock read in Heartbeat may otherwise observe a torn value. + Interlocked.Exchange(ref entry.Value.LastAccessTimestamp, Stopwatch.GetTimestamp()); } } - LimiterEntry limiterEntry = entry.Value; - // Refresh the last-access timestamp so that the Heartbeat can evict limiters whose - // IdleDuration is always null (e.g. NoopLimiter) once they stop being accessed. - limiterEntry.LastAccessTimestamp = Stopwatch.GetTimestamp(); - return limiterEntry.Limiter; + return entry.Value.Limiter; } protected override void Dispose(bool disposing) @@ -246,15 +245,17 @@ private async Task Heartbeat() // Fall back to our internally tracked last-access timestamp when the limiter // does not report an idle duration (e.g. NoopLimiter always returns null). // This ensures idle partitions are eventually evicted regardless of the limiter implementation. + // Use Interlocked.Read so the value is read atomically on 32-bit platforms where 64-bit + // reads are not guaranteed to be atomic. TimeSpan idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(limiterEntry.LastAccessTimestamp).GetValueOrDefault(); + ?? RateLimiterHelper.GetElapsedTime(Interlocked.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); if (idleDuration > s_idleTimeLimit) { lock (Lock) { // Check time again under lock to make sure no one calls Acquire or WaitAsync after checking the time and removing the limiter idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(limiterEntry.LastAccessTimestamp).GetValueOrDefault(); + ?? RateLimiterHelper.GetElapsedTime(Interlocked.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); if (idleDuration > s_idleTimeLimit) { // Remove limiter from the lookup table and mark cache as invalid From 8d250d5c0370d3cbded7c092f2db9203cac2beef Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:17:12 +0000 Subject: [PATCH 03/19] Clarify comment explaining why first-access has no eviction window Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17 Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../RateLimiting/DefaultPartitionedRateLimiter.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index 5caff3570a3dcc..af25b1016b6bb1 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -94,7 +94,10 @@ private RateLimiter GetRateLimiter(TResource resource) if (!_limiters.TryGetValue(partition.PartitionKey, out entry)) { // Using Lazy avoids calling user code (partition.Factory) inside the lock. - // The LimiterEntry constructor initializes LastAccessTimestamp when the factory runs. + // The LimiterEntry constructor initializes LastAccessTimestamp to Stopwatch.GetTimestamp() + // when the factory runs (on the first access of entry.Value below). Until then, + // Lazy.IsValueCreated is false and Heartbeat skips this entry, so there is no window + // in which the entry could be observed without a timestamp. entry = new Lazy(() => new LimiterEntry(partition.Factory(partition.PartitionKey))); _limiters.Add(partition.PartitionKey, entry); // Cache is invalid now @@ -102,10 +105,10 @@ private RateLimiter GetRateLimiter(TResource resource) } else if (entry.IsValueCreated) { - // Refresh the last-access timestamp under the lock so the Heartbeat won't - // observe a stale value and concurrently evict a limiter that's actively being used. - // Use Interlocked.Exchange so the write is atomic on 32-bit platforms where the - // outside-lock read in Heartbeat may otherwise observe a torn value. + // For subsequent accesses on an already-materialized entry, refresh the timestamp + // under the lock so the Heartbeat won't observe a stale value and concurrently evict + // a limiter that's actively being used. Use Interlocked.Exchange so the write is atomic + // on 32-bit platforms where the outside-lock read in Heartbeat may otherwise tear. Interlocked.Exchange(ref entry.Value.LastAccessTimestamp, Stopwatch.GetTimestamp()); } } From c7e37cd207a474cbaa1ed1391060bd9068206ad2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:57:38 +0000 Subject: [PATCH 04/19] Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../RateLimiting/DefaultPartitionedRateLimiter.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index af25b1016b6bb1..425ec9147c92dd 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -107,9 +107,9 @@ private RateLimiter GetRateLimiter(TResource resource) { // For subsequent accesses on an already-materialized entry, refresh the timestamp // under the lock so the Heartbeat won't observe a stale value and concurrently evict - // a limiter that's actively being used. Use Interlocked.Exchange so the write is atomic + // a limiter that's actively being used. Use Volatile.Write so the write is atomic // on 32-bit platforms where the outside-lock read in Heartbeat may otherwise tear. - Interlocked.Exchange(ref entry.Value.LastAccessTimestamp, Stopwatch.GetTimestamp()); + Volatile.Write(ref entry.Value.LastAccessTimestamp, Stopwatch.GetTimestamp()); } } @@ -248,17 +248,17 @@ private async Task Heartbeat() // Fall back to our internally tracked last-access timestamp when the limiter // does not report an idle duration (e.g. NoopLimiter always returns null). // This ensures idle partitions are eventually evicted regardless of the limiter implementation. - // Use Interlocked.Read so the value is read atomically on 32-bit platforms where 64-bit + // Use Volatile.Read so the value is read atomically on 32-bit platforms where 64-bit // reads are not guaranteed to be atomic. TimeSpan idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(Interlocked.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); + ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); if (idleDuration > s_idleTimeLimit) { lock (Lock) { // Check time again under lock to make sure no one calls Acquire or WaitAsync after checking the time and removing the limiter idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(Interlocked.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); + ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); if (idleDuration > s_idleTimeLimit) { // Remove limiter from the lookup table and mark cache as invalid From 647bd0c5cac3d2ab041b9a8677daf5e2701d1d79 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 23:19:54 +0000 Subject: [PATCH 05/19] Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValueOrDefault Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389 Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../Threading/RateLimiting/DefaultPartitionedRateLimiter.cs | 4 ++-- .../src/System/Threading/RateLimiting/RateLimiterHelper.cs | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index 425ec9147c92dd..5093e73d82cfe3 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -251,14 +251,14 @@ private async Task Heartbeat() // Use Volatile.Read so the value is read atomically on 32-bit platforms where 64-bit // reads are not guaranteed to be atomic. TimeSpan idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); + ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)); if (idleDuration > s_idleTimeLimit) { lock (Lock) { // Check time again under lock to make sure no one calls Acquire or WaitAsync after checking the time and removing the limiter idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)).GetValueOrDefault(); + ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)); if (idleDuration > s_idleTimeLimit) { // Remove limiter from the lookup table and mark cache as invalid diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs index e538fc720072dd..df3f1ecb3e0e66 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs @@ -17,6 +17,11 @@ internal static class RateLimiterHelper return Stopwatch.GetElapsedTime(startTimestamp.Value); } + public static TimeSpan GetElapsedTime(long startTimestamp) + { + return Stopwatch.GetElapsedTime(startTimestamp); + } + public static TimeSpan GetElapsedTime(long startTimestamp, long endTimestamp) { return Stopwatch.GetElapsedTime(startTimestamp, endTimestamp); From 2d2f3eda3474762dab4277bddacba36c8fc8594d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Jun 2026 02:08:58 +0000 Subject: [PATCH 06/19] Restrict idle eviction fallback to NoopLimiter Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../DefaultPartitionedRateLimiter.cs | 26 +++--- .../tests/PartitionedRateLimiterTests.cs | 84 +++++++++++-------- 2 files changed, 65 insertions(+), 45 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index 5093e73d82cfe3..8af66d6de96e17 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -245,20 +245,13 @@ private async Task Heartbeat() continue; } LimiterEntry limiterEntry = rateLimiter.Value.Value; - // Fall back to our internally tracked last-access timestamp when the limiter - // does not report an idle duration (e.g. NoopLimiter always returns null). - // This ensures idle partitions are eventually evicted regardless of the limiter implementation. - // Use Volatile.Read so the value is read atomically on 32-bit platforms where 64-bit - // reads are not guaranteed to be atomic. - TimeSpan idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)); + TimeSpan? idleDuration = GetIdleDuration(limiterEntry); if (idleDuration > s_idleTimeLimit) { lock (Lock) { // Check time again under lock to make sure no one calls Acquire or WaitAsync after checking the time and removing the limiter - idleDuration = limiterEntry.Limiter.IdleDuration - ?? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)); + idleDuration = GetIdleDuration(limiterEntry); if (idleDuration > s_idleTimeLimit) { // Remove limiter from the lookup table and mark cache as invalid @@ -307,9 +300,20 @@ private async Task Heartbeat() } } + private static TimeSpan? GetIdleDuration(LimiterEntry limiterEntry) + { + // NoopLimiter always reports IdleDuration == null, but it is also always safe to evict. + // Fall back to our internally tracked last-access timestamp only for that known case. + // Use Volatile.Read so the value is read atomically on 32-bit platforms where 64-bit + // reads are not guaranteed to be atomic. + return limiterEntry.Limiter.IdleDuration ?? (limiterEntry.Limiter is NoopLimiter + ? RateLimiterHelper.GetElapsedTime(Volatile.Read(ref limiterEntry.LastAccessTimestamp)) + : null); + } + // Wraps a RateLimiter with a timestamp of when it was last accessed by the partitioned limiter. - // The timestamp is used by the Heartbeat to evict limiters whose IdleDuration is always null - // (e.g. NoopLimiter), which would otherwise never be cleaned up. + // The timestamp is used by the Heartbeat to evict NoopLimiter partitions, whose IdleDuration + // is always null and would otherwise never be cleaned up. private sealed class LimiterEntry { public LimiterEntry(RateLimiter limiter) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index 9b7cb450046ab1..cbd4532b1cd428 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -530,23 +530,42 @@ public async Task IdleLimiterIsCleanedUp() } [Fact] - public async Task IdleLimiterWithNullIdleDurationIsCleanedUp() + public async Task NoopLimiterPartitionIsCleanedUp() + { + using var limiter = Utils.CreatePartitionedLimiterWithoutTimer(resource => + { + return RateLimitPartition.GetNoLimiter(1); + }); + + var lease = limiter.AttemptAcquire(""); + Assert.True(lease.IsAcquired); + Assert.NotNull(GetLazyLimiterEntry(limiter, key: 1)); + + // Backdate the internally-tracked last-access timestamp so the heartbeat treats the + // partition as idle even though NoopLimiter reports IdleDuration == null. + BackdateLastAccessTimestamp(limiter, key: 1); + + await Utils.RunTimerFunc(limiter); + Assert.Null(GetLazyLimiterEntry(limiter, key: 1)); + + lease = limiter.AttemptAcquire(""); + Assert.True(lease.IsAcquired); + Assert.NotNull(GetLazyLimiterEntry(limiter, key: 1)); + } + + [Fact] + public async Task LimiterWithNullIdleDurationIsNotCleanedUp() { - // Verifies that a limiter that always reports IdleDuration == null (e.g. NoopLimiter) - // is still evicted by the heartbeat once it has not been accessed for the idle window. - // Regression coverage for the NoopLimiter memory leak scenario. var factoryCallCount = 0; - CustomizableLimiter innerLimiter = null; using var limiter = Utils.CreatePartitionedLimiterWithoutTimer(resource => { return RateLimitPartition.Get(1, _ => { factoryCallCount++; - innerLimiter = new CustomizableLimiter + return new CustomizableLimiter { IdleDurationImpl = () => null }; - return innerLimiter; }); }); @@ -554,27 +573,13 @@ public async Task IdleLimiterWithNullIdleDurationIsCleanedUp() Assert.True(lease.IsAcquired); Assert.Equal(1, factoryCallCount); - var disposeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - innerLimiter.DisposeAsyncCoreImpl = () => - { - disposeTcs.SetResult(null); - return default; - }; - - // Backdate the internally-tracked last-access timestamp so the heartbeat treats the - // partition as idle even though the inner limiter reports IdleDuration == null. BackdateLastAccessTimestamp(limiter, key: 1); await Utils.RunTimerFunc(limiter); - // Limiter is disposed when timer runs and sees that the internal idle tracking is greater than the idle limit - await disposeTcs.Task; - innerLimiter.DisposeAsyncCoreImpl = () => default; - - // Acquire will call limiter factory again as the limiter was disposed and removed lease = limiter.AttemptAcquire(""); Assert.True(lease.IsAcquired); - Assert.Equal(2, factoryCallCount); + Assert.Equal(1, factoryCallCount); } // Reaches into the internal _limiters dictionary of DefaultPartitionedRateLimiter and @@ -582,27 +587,38 @@ public async Task IdleLimiterWithNullIdleDurationIsCleanedUp() // doesn't have to wait the real idle window before the heartbeat evicts the limiter. private static void BackdateLastAccessTimestamp(PartitionedRateLimiter limiter, int key) { - var limiterTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2, System.Threading.RateLimiting"); - var limiterType = limiter.GetType(); - Assert.Equal(limiterTypeDef, limiterType.GetGenericTypeDefinition()); + var entry = GetLimiterEntry(limiter, key); - var limitersField = limiterType.GetField("_limiters", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); - Assert.NotNull(limitersField); - var limitersDict = (System.Collections.IDictionary)limitersField.GetValue(limiter); - Assert.NotNull(limitersDict); + // LastAccessTimestamp is a public mutable field on the internal LimiterEntry type. + var lastAccessField = entry.GetType().GetField("LastAccessTimestamp"); + Assert.NotNull(lastAccessField); + lastAccessField.SetValue(entry, 0L); + } - var lazyEntry = limitersDict[key]; + private static object GetLimiterEntry(PartitionedRateLimiter limiter, int key) + { + var lazyEntry = GetLazyLimiterEntry(limiter, key); Assert.NotNull(lazyEntry); // Force creation of the Lazy.Value var lazyType = lazyEntry.GetType(); var entry = lazyType.GetProperty("Value").GetValue(lazyEntry); Assert.NotNull(entry); + return entry; + } - // LastAccessTimestamp is a public mutable field on the internal LimiterEntry type. - var lastAccessField = entry.GetType().GetField("LastAccessTimestamp"); - Assert.NotNull(lastAccessField); - lastAccessField.SetValue(entry, 0L); + private static object GetLazyLimiterEntry(PartitionedRateLimiter limiter, int key) + { + var limiterTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2, System.Threading.RateLimiting"); + var limiterType = limiter.GetType(); + Assert.Equal(limiterTypeDef, limiterType.GetGenericTypeDefinition()); + + var limitersField = limiterType.GetField("_limiters", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + Assert.NotNull(limitersField); + var limitersDict = (System.Collections.IDictionary)limitersField.GetValue(limiter); + Assert.NotNull(limitersDict); + + return limitersDict[key]; } [Fact] From 762a5ac029e2a6efde1b0370b5983c55ca95e4f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Jun 2026 02:13:27 +0000 Subject: [PATCH 07/19] Apply remaining changes Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- reflection_test.cs | 14 ++++++++++++++ test_dict.cs | 10 ++++++++++ test_dict.csproj | 6 ++++++ 3 files changed, 30 insertions(+) create mode 100644 reflection_test.cs create mode 100644 test_dict.cs create mode 100644 test_dict.csproj diff --git a/reflection_test.cs b/reflection_test.cs new file mode 100644 index 00000000000000..2c8c325d16a1f1 --- /dev/null +++ b/reflection_test.cs @@ -0,0 +1,14 @@ +using System; +using System.Reflection; + +class Program { + private class Inner { + public long MyField = 42; + } + + static void Main() { + object obj = new Inner(); + var field = obj.GetType().GetField("MyField"); + Console.WriteLine(field != null ? "FOUND" : "NULL"); + } +} diff --git a/test_dict.cs b/test_dict.cs new file mode 100644 index 00000000000000..3fdf9732a120e6 --- /dev/null +++ b/test_dict.cs @@ -0,0 +1,10 @@ +using System; +using System.Collections; +using System.Collections.Generic; + +class Program { + static void Main() { + IDictionary dict = new Dictionary(); + Console.WriteLine(dict[1] == null ? "NULL" : "FOUND"); + } +} diff --git a/test_dict.csproj b/test_dict.csproj new file mode 100644 index 00000000000000..dd4b56868ac6dc --- /dev/null +++ b/test_dict.csproj @@ -0,0 +1,6 @@ + + + Exe + net8.0 + + From 9e0efe95e2e87e83e65168fc87477da3251606ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Jun 2026 06:45:56 +0000 Subject: [PATCH 08/19] Merge main + fix build after Utils.RunTimerFunc signature change Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to: - Pass explicit type arguments to Utils.RunTimerFunc (signature changed in main). - Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer. All 344 tests pass; net11.0 and net481 build with 0 warnings. Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../tests/PartitionedRateLimiterTests.cs | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index cbd4532b1cd428..251972fc79030e 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -539,18 +539,18 @@ public async Task NoopLimiterPartitionIsCleanedUp() var lease = limiter.AttemptAcquire(""); Assert.True(lease.IsAcquired); - Assert.NotNull(GetLazyLimiterEntry(limiter, key: 1)); + Assert.NotNull(GetLazyLimiterEntry(limiter, key: 1)); // Backdate the internally-tracked last-access timestamp so the heartbeat treats the // partition as idle even though NoopLimiter reports IdleDuration == null. - BackdateLastAccessTimestamp(limiter, key: 1); + BackdateLastAccessTimestamp(limiter, key: 1); - await Utils.RunTimerFunc(limiter); - Assert.Null(GetLazyLimiterEntry(limiter, key: 1)); + await Utils.RunTimerFunc(limiter); + Assert.Null(GetLazyLimiterEntry(limiter, key: 1)); lease = limiter.AttemptAcquire(""); Assert.True(lease.IsAcquired); - Assert.NotNull(GetLazyLimiterEntry(limiter, key: 1)); + Assert.NotNull(GetLazyLimiterEntry(limiter, key: 1)); } [Fact] @@ -573,9 +573,9 @@ public async Task LimiterWithNullIdleDurationIsNotCleanedUp() Assert.True(lease.IsAcquired); Assert.Equal(1, factoryCallCount); - BackdateLastAccessTimestamp(limiter, key: 1); + BackdateLastAccessTimestamp(limiter, key: 1); - await Utils.RunTimerFunc(limiter); + await Utils.RunTimerFunc(limiter); lease = limiter.AttemptAcquire(""); Assert.True(lease.IsAcquired); @@ -585,33 +585,43 @@ public async Task LimiterWithNullIdleDurationIsNotCleanedUp() // Reaches into the internal _limiters dictionary of DefaultPartitionedRateLimiter and // backdates the LastAccessTimestamp on the LimiterEntry for the given key so the test // doesn't have to wait the real idle window before the heartbeat evicts the limiter. - private static void BackdateLastAccessTimestamp(PartitionedRateLimiter limiter, int key) + private static void BackdateLastAccessTimestamp(PartitionedRateLimiter limiter, TKey key) { - var entry = GetLimiterEntry(limiter, key); + // Use Type.GetType with literal strings so trimming can see what types we're reflecting on. + var limiterEntryTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2+LimiterEntry, System.Threading.RateLimiting"); + Assert.NotNull(limiterEntryTypeDef); + var limiterEntryType = limiterEntryTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); // LastAccessTimestamp is a public mutable field on the internal LimiterEntry type. - var lastAccessField = entry.GetType().GetField("LastAccessTimestamp"); + var lastAccessField = limiterEntryType.GetField("LastAccessTimestamp"); Assert.NotNull(lastAccessField); + + var entry = GetLimiterEntry(limiter, key); lastAccessField.SetValue(entry, 0L); } - private static object GetLimiterEntry(PartitionedRateLimiter limiter, int key) + private static object GetLimiterEntry(PartitionedRateLimiter limiter, TKey key) { - var lazyEntry = GetLazyLimiterEntry(limiter, key); + var lazyEntry = GetLazyLimiterEntry(limiter, key); Assert.NotNull(lazyEntry); - // Force creation of the Lazy.Value - var lazyType = lazyEntry.GetType(); + // Force creation of the Lazy.Value. Use Type.GetType with a literal string + // so trimming can see the LimiterEntry type, then construct the closed Lazy. + var limiterEntryTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2+LimiterEntry, System.Threading.RateLimiting"); + Assert.NotNull(limiterEntryTypeDef); + var limiterEntryType = limiterEntryTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); + var lazyType = typeof(Lazy<>).MakeGenericType(limiterEntryType); var entry = lazyType.GetProperty("Value").GetValue(lazyEntry); Assert.NotNull(entry); return entry; } - private static object GetLazyLimiterEntry(PartitionedRateLimiter limiter, int key) + private static object GetLazyLimiterEntry(PartitionedRateLimiter limiter, TKey key) { + // Use Type.GetType so that trimming can see what type we're reflecting on, but assert it's the one we got var limiterTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2, System.Threading.RateLimiting"); - var limiterType = limiter.GetType(); - Assert.Equal(limiterTypeDef, limiterType.GetGenericTypeDefinition()); + var limiterType = limiterTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); + Assert.Equal(limiter.GetType(), limiterType); var limitersField = limiterType.GetField("_limiters", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); Assert.NotNull(limitersField); From a6443a9f309235697eb3a9fd4d80076a438e6b4e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Jun 2026 06:46:27 +0000 Subject: [PATCH 09/19] Remove unrelated scratch files accidentally committed earlier Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- reflection_test.cs | 14 -------------- test_dict.cs | 10 ---------- test_dict.csproj | 6 ------ 3 files changed, 30 deletions(-) delete mode 100644 reflection_test.cs delete mode 100644 test_dict.cs delete mode 100644 test_dict.csproj diff --git a/reflection_test.cs b/reflection_test.cs deleted file mode 100644 index 2c8c325d16a1f1..00000000000000 --- a/reflection_test.cs +++ /dev/null @@ -1,14 +0,0 @@ -using System; -using System.Reflection; - -class Program { - private class Inner { - public long MyField = 42; - } - - static void Main() { - object obj = new Inner(); - var field = obj.GetType().GetField("MyField"); - Console.WriteLine(field != null ? "FOUND" : "NULL"); - } -} diff --git a/test_dict.cs b/test_dict.cs deleted file mode 100644 index 3fdf9732a120e6..00000000000000 --- a/test_dict.cs +++ /dev/null @@ -1,10 +0,0 @@ -using System; -using System.Collections; -using System.Collections.Generic; - -class Program { - static void Main() { - IDictionary dict = new Dictionary(); - Console.WriteLine(dict[1] == null ? "NULL" : "FOUND"); - } -} diff --git a/test_dict.csproj b/test_dict.csproj deleted file mode 100644 index dd4b56868ac6dc..00000000000000 --- a/test_dict.csproj +++ /dev/null @@ -1,6 +0,0 @@ - - - Exe - net8.0 - - From 0119d3da9b6b05b791012232cddb8be91295bb5c Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Sun, 14 Jun 2026 10:00:49 -0700 Subject: [PATCH 10/19] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../tests/PartitionedRateLimiterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index 251972fc79030e..50361c344d72b5 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -620,8 +620,8 @@ private static object GetLazyLimiterEntry(PartitionedRateLimite { // Use Type.GetType so that trimming can see what type we're reflecting on, but assert it's the one we got var limiterTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2, System.Threading.RateLimiting"); + Assert.NotNull(limiterTypeDef); var limiterType = limiterTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); - Assert.Equal(limiter.GetType(), limiterType); var limitersField = limiterType.GetField("_limiters", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); Assert.NotNull(limitersField); From cc1973100d5ba37304cc05da3312ab7bc0268dcd Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Sun, 14 Jun 2026 10:49:06 -0700 Subject: [PATCH 11/19] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../tests/PartitionedRateLimiterTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index 50361c344d72b5..78a27fc9f5b220 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -611,7 +611,9 @@ private static object GetLimiterEntry(PartitionedRateLimiter).MakeGenericType(limiterEntryType); - var entry = lazyType.GetProperty("Value").GetValue(lazyEntry); + var valueProperty = lazyType.GetProperty("Value"); + Assert.NotNull(valueProperty); + var entry = valueProperty.GetValue(lazyEntry); Assert.NotNull(entry); return entry; } From 3dec36752c92fb5a51844012e09c183db86832bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:18:53 +0000 Subject: [PATCH 12/19] Encapsulate limiter entry timestamp Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../Threading/RateLimiting/DefaultPartitionedRateLimiter.cs | 2 +- .../tests/PartitionedRateLimiterTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index 8af66d6de96e17..572145a4a92f92 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -323,7 +323,7 @@ public LimiterEntry(RateLimiter limiter) } public RateLimiter Limiter { get; } - public long LastAccessTimestamp; + internal long LastAccessTimestamp; } } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index 78a27fc9f5b220..65534b02caa1a1 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -592,8 +592,8 @@ private static void BackdateLastAccessTimestamp(PartitionedRate Assert.NotNull(limiterEntryTypeDef); var limiterEntryType = limiterEntryTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); - // LastAccessTimestamp is a public mutable field on the internal LimiterEntry type. - var lastAccessField = limiterEntryType.GetField("LastAccessTimestamp"); + // LastAccessTimestamp is a non-public mutable field on the internal LimiterEntry type. + var lastAccessField = limiterEntryType.GetField("LastAccessTimestamp", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); Assert.NotNull(lastAccessField); var entry = GetLimiterEntry(limiter, key); From 65a161d0f8c9dd2f2590fab099ace747d802ec7c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:19:28 +0000 Subject: [PATCH 13/19] Centralize reflected type names in rate limiting tests Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../tests/PartitionedRateLimiterTests.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index 65534b02caa1a1..41d9bb89c2d785 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -10,6 +10,9 @@ namespace System.Threading.RateLimiting.Tests { public class PartitionedRateLimiterTests { + private const string DefaultPartitionedRateLimiterTypeName = "System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2, System.Threading.RateLimiting"; + private const string DefaultPartitionedRateLimiterEntryTypeName = "System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2+LimiterEntry, System.Threading.RateLimiting"; + [Fact] public void ThrowsWhenAcquiringLessThanZero() { @@ -588,7 +591,7 @@ public async Task LimiterWithNullIdleDurationIsNotCleanedUp() private static void BackdateLastAccessTimestamp(PartitionedRateLimiter limiter, TKey key) { // Use Type.GetType with literal strings so trimming can see what types we're reflecting on. - var limiterEntryTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2+LimiterEntry, System.Threading.RateLimiting"); + var limiterEntryTypeDef = Type.GetType(DefaultPartitionedRateLimiterEntryTypeName); Assert.NotNull(limiterEntryTypeDef); var limiterEntryType = limiterEntryTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); @@ -607,7 +610,7 @@ private static object GetLimiterEntry(PartitionedRateLimiter.Value. Use Type.GetType with a literal string // so trimming can see the LimiterEntry type, then construct the closed Lazy. - var limiterEntryTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2+LimiterEntry, System.Threading.RateLimiting"); + var limiterEntryTypeDef = Type.GetType(DefaultPartitionedRateLimiterEntryTypeName); Assert.NotNull(limiterEntryTypeDef); var limiterEntryType = limiterEntryTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); var lazyType = typeof(Lazy<>).MakeGenericType(limiterEntryType); @@ -621,7 +624,7 @@ private static object GetLimiterEntry(PartitionedRateLimiter(PartitionedRateLimiter limiter, TKey key) { // Use Type.GetType so that trimming can see what type we're reflecting on, but assert it's the one we got - var limiterTypeDef = Type.GetType("System.Threading.RateLimiting.DefaultPartitionedRateLimiter`2, System.Threading.RateLimiting"); + var limiterTypeDef = Type.GetType(DefaultPartitionedRateLimiterTypeName); Assert.NotNull(limiterTypeDef); var limiterType = limiterTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); From c621259ca290e6a5f35d640dee917819666aa6f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:30:39 +0000 Subject: [PATCH 14/19] Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValue in GetLazyLimiterEntry Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../tests/PartitionedRateLimiterTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index 41d9bb89c2d785..c089ce65fc440b 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -630,6 +630,7 @@ private static object GetLazyLimiterEntry(PartitionedRateLimite var limitersField = limiterType.GetField("_limiters", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); Assert.NotNull(limitersField); + Assert.True(limiterType.IsInstanceOfType(limiter)); var limitersDict = (System.Collections.IDictionary)limitersField.GetValue(limiter); Assert.NotNull(limitersDict); From 56eadb632417067bb6a6f99bbc2508f19c52907a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:30:51 +0000 Subject: [PATCH 15/19] Add explicit null check for nullable TimeSpan comparison in Heartbeat Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../Threading/RateLimiting/DefaultPartitionedRateLimiter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index 572145a4a92f92..ced3592c0492cb 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -246,13 +246,13 @@ private async Task Heartbeat() } LimiterEntry limiterEntry = rateLimiter.Value.Value; TimeSpan? idleDuration = GetIdleDuration(limiterEntry); - if (idleDuration > s_idleTimeLimit) + if (idleDuration.HasValue && idleDuration.Value > s_idleTimeLimit) { lock (Lock) { // Check time again under lock to make sure no one calls Acquire or WaitAsync after checking the time and removing the limiter idleDuration = GetIdleDuration(limiterEntry); - if (idleDuration > s_idleTimeLimit) + if (idleDuration.HasValue && idleDuration.Value > s_idleTimeLimit) { // Remove limiter from the lookup table and mark cache as invalid // If a request for this partition comes in it will have to create a new limiter now From 97da373b9c0a11b0ca1b3a7d91893219e5993456 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:29:23 +0000 Subject: [PATCH 16/19] Clarify RateLimiting test comment Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../tests/PartitionedRateLimiterTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index c089ce65fc440b..f0ee3a25436242 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -623,7 +623,8 @@ private static object GetLimiterEntry(PartitionedRateLimiter(PartitionedRateLimiter limiter, TKey key) { - // Use Type.GetType so that trimming can see what type we're reflecting on, but assert it's the one we got + // Use Type.GetType so trimming can see what type we're reflecting on, then construct the + // closed DefaultPartitionedRateLimiter type for the requested arguments. var limiterTypeDef = Type.GetType(DefaultPartitionedRateLimiterTypeName); Assert.NotNull(limiterTypeDef); var limiterType = limiterTypeDef.MakeGenericType(typeof(TResource), typeof(TKey)); From 72f0a07bf86aff254efd73145f577ad970ff6d1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:32:29 +0000 Subject: [PATCH 17/19] Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../RateLimiting/DefaultPartitionedRateLimiter.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index ced3592c0492cb..6fe104572b512b 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -245,14 +245,13 @@ private async Task Heartbeat() continue; } LimiterEntry limiterEntry = rateLimiter.Value.Value; - TimeSpan? idleDuration = GetIdleDuration(limiterEntry); - if (idleDuration.HasValue && idleDuration.Value > s_idleTimeLimit) + if (GetIdleDuration(limiterEntry) is TimeSpan idleDuration && idleDuration > s_idleTimeLimit) { lock (Lock) { // Check time again under lock to make sure no one calls Acquire or WaitAsync after checking the time and removing the limiter - idleDuration = GetIdleDuration(limiterEntry); - if (idleDuration.HasValue && idleDuration.Value > s_idleTimeLimit) + idleDuration = GetIdleDuration(limiterEntry) ?? TimeSpan.Zero; + if (idleDuration > s_idleTimeLimit) { // Remove limiter from the lookup table and mark cache as invalid // If a request for this partition comes in it will have to create a new limiter now From 7e6354daa7251314940e9af627a3a6259c5f5ee8 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Wed, 17 Jun 2026 11:34:29 -0700 Subject: [PATCH 18/19] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../RateLimiting/DefaultPartitionedRateLimiter.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs index 6fe104572b512b..f9cf3393698c1b 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs @@ -105,11 +105,14 @@ private RateLimiter GetRateLimiter(TResource resource) } else if (entry.IsValueCreated) { - // For subsequent accesses on an already-materialized entry, refresh the timestamp - // under the lock so the Heartbeat won't observe a stale value and concurrently evict - // a limiter that's actively being used. Use Volatile.Write so the write is atomic - // on 32-bit platforms where the outside-lock read in Heartbeat may otherwise tear. - Volatile.Write(ref entry.Value.LastAccessTimestamp, Stopwatch.GetTimestamp()); + LimiterEntry limiterEntry = entry.Value; + + if (limiterEntry.Limiter is NoopLimiter) + { + // Refresh the timestamp under the lock so Heartbeat won't evict a limiter that's actively being used. + // Use Volatile.Write so the write is atomic on 32-bit platforms where the outside-lock read in Heartbeat may otherwise tear. + Volatile.Write(ref limiterEntry.LastAccessTimestamp, Stopwatch.GetTimestamp()); + } } } From ff41bbc0f5d357a20b02477d0cbaf6c7fbe97b25 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Wed, 17 Jun 2026 21:26:32 -0700 Subject: [PATCH 19/19] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../tests/PartitionedRateLimiterTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs index f0ee3a25436242..2c86be1bb781e6 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs @@ -600,7 +600,8 @@ private static void BackdateLastAccessTimestamp(PartitionedRate Assert.NotNull(lastAccessField); var entry = GetLimiterEntry(limiter, key); - lastAccessField.SetValue(entry, 0L); + long backdatedTimestamp = System.Diagnostics.Stopwatch.GetTimestamp() - (System.Diagnostics.Stopwatch.Frequency * 60); + lastAccessField.SetValue(entry, backdatedTimestamp); } private static object GetLimiterEntry(PartitionedRateLimiter limiter, TKey key)