From a566260eceb8b461b1b20a482924c9e78f1de7c9 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 5 Apr 2021 07:52:08 -0700 Subject: [PATCH 1/3] Re-use cancellation tokens in the https middleware - These tokens are usually short lived and exist for the purposes of cancelling the handshake. --- .../Internal/CancellationTokenSourcePool.cs | 62 +++++++++++++++++++ .../Middleware/HttpsConnectionMiddleware.cs | 7 ++- 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs b/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs new file mode 100644 index 000000000000..54c539b9fd08 --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs @@ -0,0 +1,62 @@ +using System.Collections.Concurrent; +using System.Threading; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal +{ + internal class CancellationTokenSourcePool + { + private const int MaxQueueSize = 1024; + + private readonly ConcurrentQueue _queue = new(); + private int _count; + + public PooledCancellationTokenSource Rent() + { + if (_queue.TryDequeue(out var cts)) + { + Interlocked.Decrement(ref _count); + return cts; + } + return new PooledCancellationTokenSource(this); + } + + private bool Return(PooledCancellationTokenSource cts) + { + // This counting isn't accurate, but it's good enough for what we need to avoid using _queue.Count which could be expensive + if (!cts.TryReset() || Interlocked.Increment(ref _count) > MaxQueueSize) + { + Interlocked.Decrement(ref _count); + return false; + } + + _queue.Enqueue(cts); + return true; + } + + /// + /// A with a back pointer to the pool it came from. + /// Dispose will return it to the pool. + /// + public class PooledCancellationTokenSource : CancellationTokenSource + { + private readonly CancellationTokenSourcePool _pool; + + public PooledCancellationTokenSource(CancellationTokenSourcePool pool) + { + _pool = pool; + } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + // If we failed to return to the pool then dispose + if (!_pool.Return(this)) + { + base.Dispose(disposing); + } + } + } + } + } +} diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index 6742972ead45..501fa13a0a52 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -49,6 +49,9 @@ internal class HttpsConnectionMiddleware private readonly HttpsOptionsCallback? _httpsOptionsCallback; private readonly object? _httpsOptionsCallbackState; + // Pool for cancellation tokens that cancel the handshake + private readonly CancellationTokenSourcePool _ctsPool = new(); + public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options) : this(next, options, loggerFactory: NullLoggerFactory.Instance) { @@ -150,7 +153,9 @@ public async Task OnConnectionAsync(ConnectionContext context) try { - using var cancellationTokenSource = new CancellationTokenSource(_handshakeTimeout); + using var cancellationTokenSource = _ctsPool.Rent(); + cancellationTokenSource.CancelAfter(_handshakeTimeout); + if (_httpsOptionsCallback is null) { await DoOptionsBasedHandshakeAsync(context, sslStream, feature, cancellationTokenSource.Token); From 0f69788d573c01650051f1ad81a17697cab2ad57 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 5 Apr 2021 13:51:44 -0700 Subject: [PATCH 2/3] Added copyright --- .../Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs b/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs index 54c539b9fd08..2efe8995e459 100644 --- a/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs +++ b/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs @@ -1,3 +1,6 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + using System.Collections.Concurrent; using System.Threading; From daa0cef0f191db74de1e4623bfba796fa9ecbdf6 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 5 Apr 2021 20:07:17 -0700 Subject: [PATCH 3/3] PR feedback --- .../Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs b/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs index 2efe8995e459..ffb3a30e558a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs +++ b/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs @@ -25,8 +25,7 @@ public PooledCancellationTokenSource Rent() private bool Return(PooledCancellationTokenSource cts) { - // This counting isn't accurate, but it's good enough for what we need to avoid using _queue.Count which could be expensive - if (!cts.TryReset() || Interlocked.Increment(ref _count) > MaxQueueSize) + if (Interlocked.Increment(ref _count) > MaxQueueSize || !cts.TryReset()) { Interlocked.Decrement(ref _count); return false;