diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ChainedPartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ChainedPartitionedRateLimiter.cs index f3047b941f9d5..91ebe1126d9c9 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ChainedPartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ChainedPartitionedRateLimiter.cs @@ -29,7 +29,7 @@ public ChainedPartitionedRateLimiter(PartitionedRateLimiter[] limiter long lowestAvailablePermits = long.MaxValue; long currentQueuedCount = 0; long totalFailedLeases = 0; - long totalSuccessfulLeases = 0; + long innerMostSuccessfulLeases = 0; foreach (PartitionedRateLimiter limiter in _limiters) { if (limiter.GetStatistics(resource) is { } statistics) @@ -40,7 +40,7 @@ public ChainedPartitionedRateLimiter(PartitionedRateLimiter[] limiter } currentQueuedCount += statistics.CurrentQueuedCount; totalFailedLeases += statistics.TotalFailedLeases; - totalSuccessfulLeases += statistics.TotalSuccessfulLeases; + innerMostSuccessfulLeases = statistics.TotalSuccessfulLeases; } } return new RateLimiterStatistics() @@ -48,7 +48,7 @@ public ChainedPartitionedRateLimiter(PartitionedRateLimiter[] limiter CurrentAvailablePermits = lowestAvailablePermits, CurrentQueuedCount = currentQueuedCount, TotalFailedLeases = totalFailedLeases, - TotalSuccessfulLeases = totalSuccessfulLeases, + TotalSuccessfulLeases = innerMostSuccessfulLeases, }; } diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ConcurrencyLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ConcurrencyLimiter.cs index 0b0e5e8cfeaac..508340a2e138b 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ConcurrencyLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ConcurrencyLimiter.cs @@ -67,6 +67,7 @@ public ConcurrencyLimiter(ConcurrencyLimiterOptions options) /// public override RateLimiterStatistics? GetStatistics() { + ThrowIfDisposed(); return new RateLimiterStatistics() { CurrentAvailablePermits = _permitCount, diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs index 651db82896ba5..fe4b0c29c3a62 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs @@ -86,6 +86,7 @@ public FixedWindowRateLimiter(FixedWindowRateLimiterOptions options) /// public override RateLimiterStatistics? GetStatistics() { + ThrowIfDisposed(); return new RateLimiterStatistics() { CurrentAvailablePermits = _requestCount, diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.T.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.T.cs index ee96362347b68..b97ac2de2c1cb 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.T.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.T.cs @@ -126,16 +126,14 @@ public async ValueTask DisposeAsync() /// /// The type to translate into . /// The function to be called every time a is passed to - /// PartitionedRateLimiter<TOuter>.Acquire(TOuter, int) or PartitionedRateLimiter<TOuter>.WaitAsync(TOuter, int, CancellationToken). - /// should be implemented in a thread-safe way. + /// PartitionedRateLimiter<TOuter>.Acquire(TOuter, int) or PartitionedRateLimiter<TOuter>.WaitAsync(TOuter, int, CancellationToken). + /// + /// should be implemented in a thread-safe way. /// Specifies whether the returned will dispose the wrapped . /// A new PartitionedRateLimiter<TOuter> that translates /// to and calls the inner . public PartitionedRateLimiter WithTranslatedKey(Func 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(this, keyAdapter, leaveOpen); } } diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.cs index 2bfe22e3f9496..a2f592c5ea3b5 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/PartitionedRateLimiter.cs @@ -38,7 +38,8 @@ public static PartitionedRateLimiter Create /// Methods on the returned will iterate over the passed in in the order given. /// /// - /// will return the lowest value for + /// will return the lowest value for , + /// the inner-most limiter's , /// and the aggregate values for the rest of the properties from the . /// /// diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs index 6b3fd0955c330..1ccf40775e2d8 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs @@ -94,6 +94,7 @@ public SlidingWindowRateLimiter(SlidingWindowRateLimiterOptions options) /// public override RateLimiterStatistics? GetStatistics() { + ThrowIfDisposed(); return new RateLimiterStatistics() { CurrentAvailablePermits = _requestCount, diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs index 11b043dde3aeb..7baf91ea59080 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs @@ -88,6 +88,7 @@ public TokenBucketRateLimiter(TokenBucketRateLimiterOptions options) /// public override RateLimiterStatistics? GetStatistics() { + ThrowIfDisposed(); return new RateLimiterStatistics() { CurrentAvailablePermits = _tokenCount, diff --git a/src/libraries/System.Threading.RateLimiting/tests/BaseRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/BaseRateLimiterTests.cs index 83c9a118cc4f6..e4adf232b9b5a 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/BaseRateLimiterTests.cs +++ b/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(); } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/ChainedLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/ChainedLimiterTests.cs index 533364137ae46..7310f2417531e 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/ChainedLimiterTests.cs +++ b/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(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); } diff --git a/src/libraries/System.Threading.RateLimiting/tests/ConcurrencyLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/ConcurrencyLimiterTests.cs index ff260f0e2cd9d..54e2bbecc1975 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/ConcurrencyLimiterTests.cs +++ b/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(limiter.GetStatistics); + } } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs index d21ced08b7936..6830a1ce74281 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs +++ b/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(limiter.GetStatistics); + } } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs index f8eb545f7ede7..7241a39e0f1f4 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs +++ b/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(limiter.GetStatistics); + } } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs index 278c114e64fa4..272c294a09b34 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs +++ b/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(limiter.GetStatistics); + } } }