fb

src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ChainedPartitionedRateLimiter.cs

@@ -29,7 +29,7 @@ public ChainedPartitionedRateLimiter(PartitionedRateLimiter<TResource>[] limiter
             long lowestAvailablePermits = long.MaxValue;
             long currentQueuedCount = 0;
             long totalFailedLeases = 0;
-            long totalSuccessfulLeases = 0;
+            long innerMostSuccessfulLeases = 0;
             foreach (PartitionedRateLimiter<TResource> limiter in _limiters)
             {
                 if (limiter.GetStatistics(resource) is { } statistics)
@@ -40,15 +40,15 @@ public ChainedPartitionedRateLimiter(PartitionedRateLimiter<TResource>[] limiter
                     }
                     currentQueuedCount += statistics.CurrentQueuedCount;
                     totalFailedLeases += statistics.TotalFailedLeases;
-                    totalSuccessfulLeases += statistics.TotalSuccessfulLeases;
+                    innerMostSuccessfulLeases = statistics.TotalSuccessfulLeases;
                 }
             }
             return new RateLimiterStatistics()
             {
                 CurrentAvailablePermits = lowestAvailablePermits,
                 CurrentQueuedCount = currentQueuedCount,
                 TotalFailedLeases = totalFailedLeases,
-                TotalSuccessfulLeases = totalSuccessfulLeases,
+                TotalSuccessfulLeases = innerMostSuccessfulLeases,
             };
         }
 

src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ConcurrencyLimiter.cs

@@ -67,6 +67,7 @@ public ConcurrencyLimiter(ConcurrencyLimiterOptions options)
         /// <inheritdoc/>
         public override RateLimiterStatistics? GetStatistics()
         {
+            ThrowIfDisposed();
             return new RateLimiterStatistics()
             {
                 CurrentAvailablePermits = _permitCount,

src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs

@@ -86,6 +86,7 @@ public FixedWindowRateLimiter(FixedWindowRateLimiterOptions options)
         /// <inheritdoc/>
         public override RateLimiterStatistics? GetStatistics()
         {
+            ThrowIfDisposed();
             return new RateLimiterStatistics()
             {
                 CurrentAvailablePermits = _requestCount,

src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.T.cs

@@ -126,16 +126,14 @@ public async ValueTask DisposeAsync()
         /// </summary>
         /// <typeparam name="TOuter">The type to translate into <typeparamref name="TResource"/>.</typeparam>
         /// <param name="keyAdapter">The function to be called every time a <typeparamref name="TOuter"/> is passed to
-        /// PartitionedRateLimiter&lt;TOuter&gt;.Acquire(TOuter, int) or PartitionedRateLimiter&lt;TOuter&gt;.WaitAsync(TOuter, int, CancellationToken).</param>
-        /// <remarks><paramref name="keyAdapter"/> should be implemented in a thread-safe way.
+        /// PartitionedRateLimiter&lt;TOuter&gt;.Acquire(TOuter, int) or PartitionedRateLimiter&lt;TOuter&gt;.WaitAsync(TOuter, int, CancellationToken).
+        /// <para />
+        /// <remarks><paramref name="keyAdapter"/> should be implemented in a thread-safe way.</remarks></param>
         /// <param name="leaveOpen">Specifies whether the returned <see cref="PartitionedRateLimiter{TOuter}"/> will dispose the wrapped <see cref="PartitionedRateLimiter{TResource}"/>.</param>
         /// <returns>A new PartitionedRateLimiter&lt;TOuter&gt; that translates <typeparamref name="TOuter"/>
         /// to <typeparamref name="TResource"/> and calls the inner <see cref="PartitionedRateLimiter{TResource}"/>.</returns>
         public PartitionedRateLimiter<TOuter> WithTranslatedKey<TOuter>(Func<TOuter, TResource> keyAdapter, bool leaveOpen)
         {
-            // REVIEW: Do we want to have an option to dispose the inner limiter?
-            // Should the default be to dispose the inner limiter and have an option to not dispose it?
-            // See Stream wrappers like SslStream for prior-art
             return new TranslatingLimiter<TResource, TOuter>(this, keyAdapter, leaveOpen);
         }
     }

src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.cs

@@ -38,7 +38,8 @@ public static PartitionedRateLimiter<TResource> Create<TResource, TPartitionKey>
         /// Methods on the returned <see cref="PartitionedRateLimiter{TResource}"/> will iterate over the passed in <paramref name="limiters"/> in the order given.
         /// </para>
         /// <para>
-        /// <see cref="PartitionedRateLimiter{TResource}.GetStatistics(TResource)"/> will return the lowest value for <see cref="RateLimiterStatistics.CurrentAvailablePermits"/>
+        /// <see cref="PartitionedRateLimiter{TResource}.GetStatistics(TResource)"/> will return the lowest value for <see cref="RateLimiterStatistics.CurrentAvailablePermits"/>,
+        /// the inner-most limiter's <see cref="RateLimiterStatistics.TotalSuccessfulLeases"/>,
         /// and the aggregate values for the rest of the properties from the <paramref name="limiters"/>.
         /// </para>
         /// <para>

src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs

@@ -94,6 +94,7 @@ public SlidingWindowRateLimiter(SlidingWindowRateLimiterOptions options)
         /// <inheritdoc/>
         public override RateLimiterStatistics? GetStatistics()
         {
+            ThrowIfDisposed();
             return new RateLimiterStatistics()
             {
                 CurrentAvailablePermits = _requestCount,

src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs

@@ -88,6 +88,7 @@ public TokenBucketRateLimiter(TokenBucketRateLimiterOptions options)
         /// <inheritdoc/>
         public override RateLimiterStatistics? GetStatistics()
         {
+            ThrowIfDisposed();
             return new RateLimiterStatistics()
             {
                 CurrentAvailablePermits = _tokenCount,

src/libraries/System.Threading.RateLimiting/tests/BaseRateLimiterTests.cs

@@ -124,5 +124,8 @@ public abstract class BaseRateLimiterTests
 
         [Fact]
         public abstract Task GetStatisticsWithZeroPermitCount();
+
+        [Fact]
+        public abstract void GetStatisticsThrowsAfterDispose();
     }
 }

src/libraries/System.Threading.RateLimiting/tests/ChainedLimiterTests.cs

@@ -194,7 +194,7 @@ public void GetStatisticsReturnsNewInstances()
         }
 
         [Fact]
-        public void GetStatisticsHasCorrectValues()
+        public async Task GetStatisticsHasCorrectValues()
         {
             using var limiter1 = PartitionedRateLimiter.Create<string, int>(resource =>
             {
@@ -231,7 +231,7 @@ public void GetStatisticsHasCorrectValues()
 
             Assert.Equal(3, stats.CurrentAvailablePermits);
             Assert.Equal(0, stats.CurrentQueuedCount);
-            Assert.Equal(3, stats.TotalSuccessfulLeases);
+            Assert.Equal(1, stats.TotalSuccessfulLeases);
             Assert.Equal(0, stats.TotalFailedLeases);
 
             var lease2 = chainedLimiter.AttemptAcquire("", 10);
@@ -240,7 +240,7 @@ public void GetStatisticsHasCorrectValues()
 
             Assert.Equal(3, stats.CurrentAvailablePermits);
             Assert.Equal(0, stats.CurrentQueuedCount);
-            Assert.Equal(5, stats.TotalSuccessfulLeases);
+            Assert.Equal(1, stats.TotalSuccessfulLeases);
             Assert.Equal(1, stats.TotalFailedLeases);
 
             var task = chainedLimiter.AcquireAsync("", 10);
@@ -249,7 +249,18 @@ public void GetStatisticsHasCorrectValues()
 
             Assert.Equal(2, stats.CurrentAvailablePermits);
             Assert.Equal(10, stats.CurrentQueuedCount);
-            Assert.Equal(7, stats.TotalSuccessfulLeases);
+            Assert.Equal(1, stats.TotalSuccessfulLeases);
+            Assert.Equal(1, stats.TotalFailedLeases);
+
+            lease.Dispose();
+
+            lease = await task;
+            Assert.True(lease.IsAcquired);
+            stats = chainedLimiter.GetStatistics("");
+
+            Assert.Equal(3, stats.CurrentAvailablePermits);
+            Assert.Equal(0, stats.CurrentQueuedCount);
+            Assert.Equal(2, stats.TotalSuccessfulLeases);
             Assert.Equal(1, stats.TotalFailedLeases);
         }
 

src/libraries/System.Threading.RateLimiting/tests/ConcurrencyLimiterTests.cs

@@ -917,5 +917,18 @@ public override async Task GetStatisticsWithZeroPermitCount()
             Assert.Equal(1, limiter.GetStatistics().TotalFailedLeases);
             Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits);
         }
+
+        [Fact]
+        public override void GetStatisticsThrowsAfterDispose()
+        {
+            var limiter = new ConcurrencyLimiter(new ConcurrencyLimiterOptions
+            {
+                PermitLimit = 100,
+                QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
+                QueueLimit = 50
+            });
+            limiter.Dispose();
+            Assert.Throws<ObjectDisposedException>(limiter.GetStatistics);
+        }
     }
 }

src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs

@@ -1136,5 +1136,20 @@ public override async Task GetStatisticsWithZeroPermitCount()
             Assert.Equal(1, limiter.GetStatistics().TotalFailedLeases);
             Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits);
         }
+
+        [Fact]
+        public override void GetStatisticsThrowsAfterDispose()
+        {
+            var limiter = new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions
+            {
+                PermitLimit = 100,
+                QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
+                QueueLimit = 50,
+                Window = TimeSpan.Zero,
+                AutoReplenishment = false
+            });
+            limiter.Dispose();
+            Assert.Throws<ObjectDisposedException>(limiter.GetStatistics);
+        }
     }
 }

src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs

@@ -1199,5 +1199,21 @@ public override async Task GetStatisticsWithZeroPermitCount()
             Assert.Equal(1, limiter.GetStatistics().TotalFailedLeases);
             Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits);
         }
+
+        [Fact]
+        public override void GetStatisticsThrowsAfterDispose()
+        {
+            var limiter = new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions
+            {
+                PermitLimit = 100,
+                QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
+                QueueLimit = 50,
+                Window = TimeSpan.Zero,
+                SegmentsPerWindow = 3,
+                AutoReplenishment = false
+            });
+            limiter.Dispose();
+            Assert.Throws<ObjectDisposedException>(limiter.GetStatistics);
+        }
     }
 }

src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs

@@ -1322,5 +1322,21 @@ public override async Task GetStatisticsWithZeroPermitCount()
             Assert.Equal(1, limiter.GetStatistics().TotalFailedLeases);
             Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits);
         }
+
+        [Fact]
+        public override void GetStatisticsThrowsAfterDispose()
+        {
+            var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions
+            {
+                TokenLimit = 100,
+                QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
+                QueueLimit = 50,
+                ReplenishmentPeriod = TimeSpan.Zero,
+                TokensPerPeriod = 30,
+                AutoReplenishment = false
+            });
+            limiter.Dispose();
+            Assert.Throws<ObjectDisposedException>(limiter.GetStatistics);
+        }
     }
 }