diff --git a/src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterMiddleware.cs b/src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterMiddleware.cs index 16c612cae9..394be846dc 100644 --- a/src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterMiddleware.cs +++ b/src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterMiddleware.cs @@ -96,7 +96,7 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter LoggerMessage.Define(LogLevel.Debug, new EventId(3, "RequestRunImmediately"), "Below MaxConcurrentRequests limit, running request immediately. Current active requests: {ActiveRequests}"); private static readonly Action _requestRejectedQueueFull = - LoggerMessage.Define(LogLevel.Debug, new EventId(4, "RequestRejectedQueueFull"), "Currently at the 'RequestQueueLimit', rejecting this request with a '503 server not availible' error"); + LoggerMessage.Define(LogLevel.Debug, new EventId(4, "RequestRejectedQueueFull"), "Currently at the 'RequestQueueLimit', rejecting this request with a '503 server not available' error"); internal static void RequestEnqueued(ILogger logger, int activeRequests) { diff --git a/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicy.cs b/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicy.cs index 08d6338df2..09d378e4e1 100644 --- a/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicy.cs +++ b/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicy.cs @@ -13,9 +13,9 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter private readonly int _maxTotalRequest; private readonly SemaphoreSlim _serverSemaphore; - private object _totalRequestsLock = new object(); + private int _totalRequests; - public int TotalRequests { get; private set; } + public int TotalRequests => _totalRequests; public QueuePolicy(IOptions options) { @@ -44,14 +44,12 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter // a return value of 'true' indicates that the request may proceed // _serverSemaphore.Release is *not* called in this method, it is called externally when requests leave the server - lock (_totalRequestsLock) - { - if (TotalRequests >= _maxTotalRequest) - { - return new ValueTask(false); - } + int totalRequests = Interlocked.Increment(ref _totalRequests); - TotalRequests++; + if (totalRequests > _maxTotalRequest) + { + Interlocked.Decrement(ref _totalRequests); + return new ValueTask(false); } Task task = _serverSemaphore.WaitAsync(); @@ -67,10 +65,7 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter { _serverSemaphore.Release(); - lock (_totalRequestsLock) - { - TotalRequests--; - } + Interlocked.Decrement(ref _totalRequests); } public void Dispose() diff --git a/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicyOptions.cs b/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicyOptions.cs index 2315a6bdeb..5e5eaec3b7 100644 --- a/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicyOptions.cs +++ b/src/Middleware/ConcurrencyLimiter/src/QueuePolicies/QueuePolicyOptions.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter public int MaxConcurrentRequests { get; set; } /// - /// Maximum number of queued requests before the server starts rejecting connections with '503 Service Unavailible'. + /// Maximum number of queued requests before the server starts rejecting connections with '503 Service Unavailable'. /// This option is highly application dependant, and must be configured by the application. /// public int RequestQueueLimit { get; set; } diff --git a/src/Middleware/ConcurrencyLimiter/test/PolicyTests/QueuePolicyTests.cs b/src/Middleware/ConcurrencyLimiter/test/PolicyTests/QueuePolicyTests.cs index cc8c40457c..8d569322b7 100644 --- a/src/Middleware/ConcurrencyLimiter/test/PolicyTests/QueuePolicyTests.cs +++ b/src/Middleware/ConcurrencyLimiter/test/PolicyTests/QueuePolicyTests.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter.Tests.PolicyTests public class QueuePolicyTests { [Fact] - public void DoesNotWaitIfSpaceAvailible() + public void DoesNotWaitIfSpaceAvailable() { using var s = TestUtils.CreateQueuePolicy(2); @@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter.Tests.PolicyTests } [Fact] - public async Task WaitsIfNoSpaceAvailible() + public async Task WaitsIfNoSpaceAvailable() { using var s = TestUtils.CreateQueuePolicy(1); Assert.True(await s.TryEnterAsync().OrTimeout()); @@ -36,6 +36,27 @@ namespace Microsoft.AspNetCore.ConcurrencyLimiter.Tests.PolicyTests Assert.True(await waitingTask.OrTimeout()); } + [Fact] + public void DoesNotWaitIfQueueFull() + { + using var s = TestUtils.CreateQueuePolicy(2, 1); + + var t1 = s.TryEnterAsync(); + Assert.True(t1.IsCompleted); + Assert.True(t1.Result); + + var t2 = s.TryEnterAsync(); + Assert.True(t2.IsCompleted); + Assert.True(t2.Result); + + var t3 = s.TryEnterAsync(); + Assert.False(t3.IsCompleted); + + var t4 = s.TryEnterAsync(); + Assert.True(t4.IsCompleted); + Assert.False(t4.Result); + } + [Fact] public async Task IsEncapsulated() {