diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 698b392b3e..c64db15642 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -72,6 +72,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 internal const int InitialStreamPoolSize = 5; internal const int MaxStreamPoolSize = 100; + internal const long StreamPoolExpiryTicks = TimeSpan.TicksPerSecond * 5; public Http2Connection(HttpConnectionContext context) { @@ -218,6 +219,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 // Call UpdateCompletedStreams() prior to frame processing in order to remove any streams that have exceeded their drain timeouts. UpdateCompletedStreams(); + if (result.IsCanceled) + { + // Heartbeat will cancel ReadAsync and trigger expiring unused streams from pool. + StreamPool.RemoveExpired(SystemClock.UtcNowTicks); + } + try { bool frameReceived = false; @@ -647,6 +654,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 if (StreamPool.Count < MaxStreamPoolSize) { + // This property is used to remove unused streams from the pool + stream.DrainExpirationTicks = SystemClock.UtcNowTicks + StreamPoolExpiryTicks; + StreamPool.Push(stream); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamStack.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamStack.cs index 88512a19df..5b3abac1c9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamStack.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamStack.cs @@ -9,7 +9,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 // See https://github.com/dotnet/runtime/blob/master/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegmentStack.cs internal struct Http2StreamStack { - private Http2StreamAsValueType[] _array; + // Internal for testing + internal Http2StreamAsValueType[] _array; private int _size; public Http2StreamStack(int size) @@ -78,7 +79,59 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _size++; } - private readonly struct Http2StreamAsValueType + public void RemoveExpired(long now) + { + int size = _size; + Http2StreamAsValueType[] array = _array; + + var removeCount = CalculateRemoveCount(now, size, array); + if (removeCount == 0) + { + return; + } + + var newSize = size - removeCount; + + // Dispose removed streams + for (var i = 0; i < removeCount; i++) + { + Http2Stream stream = array[i]; + stream.Dispose(); + } + + // Move remaining streams + for (var i = 0; i < newSize; i++) + { + array[i] = array[i + removeCount]; + } + + // Clear unused array indexes + for (var i = newSize; i < size; i++) + { + array[i] = default; + } + + _size = newSize; + } + + private static int CalculateRemoveCount(long now, int size, Http2StreamAsValueType[] array) + { + for (var i = 0; i < size; i++) + { + Http2Stream stream = array[i]; + if (stream.DrainExpirationTicks >= now) + { + // Stream is still valid. All streams after this will have a later expiration. + // No reason to keep checking. Return count of streams to remove. + return i; + } + } + + // All will be removed. + return size; + } + + internal readonly struct Http2StreamAsValueType { private readonly Http2Stream _value; private Http2StreamAsValueType(Http2Stream value) => _value = value; diff --git a/src/Servers/Kestrel/Core/test/Http2StreamStackTests.cs b/src/Servers/Kestrel/Core/test/Http2StreamStackTests.cs new file mode 100644 index 0000000000..2618babfd2 --- /dev/null +++ b/src/Servers/Kestrel/Core/test/Http2StreamStackTests.cs @@ -0,0 +1,113 @@ +// 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 Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; +using Microsoft.AspNetCore.Testing; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class Http2StreamStackTests + { + [Fact] + public void RemoveExpired_Empty_NoOp() + { + var streams = new Http2StreamStack(10); + + streams.RemoveExpired(100); + } + + [Fact] + public void RemoveExpired_NoneExpired_NoOp() + { + var streams = new Http2StreamStack(10); + streams.Push(CreateStream(streamId: 1, expirationTicks: 200)); + + streams.RemoveExpired(100); + + Assert.Equal(1, streams.Count); + Assert.Equal(1, ((Http2Stream)streams._array[0]).StreamId); + } + + [Fact] + public void RemoveExpired_OneExpired_ExpiredStreamRemoved() + { + var streams = new Http2StreamStack(10); + streams.Push(CreateStream(streamId: 1, expirationTicks: 200)); + + streams.RemoveExpired(300); + + Assert.Equal(0, streams.Count); + Assert.Equal(default, streams._array[0]); + } + + [Fact] + public void RemoveExpired_MultipleExpired_ExpiredStreamsRemoved() + { + var streams = new Http2StreamStack(10); + streams.Push(CreateStream(streamId: 1, expirationTicks: 200)); + streams.Push(CreateStream(streamId: 2, expirationTicks: 250)); + + streams.RemoveExpired(300); + + Assert.Equal(0, streams.Count); + Assert.Equal(default, streams._array[0]); + Assert.Equal(default, streams._array[1]); + } + + [Fact] + public void RemoveExpired_OneExpiredAndOneValid_ExpiredStreamRemoved() + { + var streams = new Http2StreamStack(10); + streams.Push(CreateStream(streamId: 1, expirationTicks: 200)); + streams.Push(CreateStream(streamId: 2, expirationTicks: 400)); + + streams.RemoveExpired(300); + + Assert.Equal(1, streams.Count); + Assert.Equal(2, ((Http2Stream)streams._array[0]).StreamId); + Assert.Equal(default, streams._array[1]); + } + + [Fact] + public void RemoveExpired_AllExpired_ExpiredStreamRemoved() + { + var streams = new Http2StreamStack(5); + streams.Push(CreateStream(streamId: 1, expirationTicks: 200)); + streams.Push(CreateStream(streamId: 2, expirationTicks: 200)); + streams.Push(CreateStream(streamId: 3, expirationTicks: 200)); + streams.Push(CreateStream(streamId: 4, expirationTicks: 200)); + streams.Push(CreateStream(streamId: 5, expirationTicks: 200)); + + streams.RemoveExpired(300); + + Assert.Equal(0, streams.Count); + Assert.Equal(5, streams._array.Length); + Assert.Equal(default, streams._array[0]); + Assert.Equal(default, streams._array[1]); + Assert.Equal(default, streams._array[2]); + Assert.Equal(default, streams._array[3]); + Assert.Equal(default, streams._array[4]); + } + + private static Http2Stream CreateStream(int streamId, long expirationTicks) + { + var context = new Http2StreamContext + { + StreamId = streamId, + ServiceContext = new Internal.ServiceContext + { + ServerOptions = new KestrelServerOptions() + }, + ServerPeerSettings = new Http2PeerSettings(), + ClientPeerSettings = new Http2PeerSettings() + }; + + return new Http2Stream(new DummyApplication(), context) + { + DrainExpirationTicks = expirationTicks + }; + } + } +} diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index a969bfe609..19a7809a40 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -478,7 +478,58 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(0, _connection.StreamPool.Count); await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false); + } + [Fact] + public async Task StreamPool_UnusedExpiredStream_RemovedFromPool() + { + DateTimeOffset now = _serviceContext.MockSystemClock.UtcNow; + + // Heartbeat + TriggerTick(now); + + var serverTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await InitializeConnectionAsync(async context => + { + await _echoApplication(context); + }); + + Assert.Equal(0, _connection.StreamPool.Count); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 36, + withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM), + withStreamId: 1); + + // Ping will trigger the stream to be returned to the pool so we can assert it + await SendPingAsync(Http2PingFrameFlags.NONE); + await ExpectAsync(Http2FrameType.PING, + withLength: 8, + withFlags: (byte)Http2PingFrameFlags.ACK, + withStreamId: 0); + + // Stream has been returned to the pool + Assert.Equal(1, _connection.StreamPool.Count); + + _connection.StreamPool.TryPeek(out var pooledStream); + + TriggerTick(now + TimeSpan.FromSeconds(1)); + + // Stream has not expired and is still in pool + Assert.Equal(1, _connection.StreamPool.Count); + + TriggerTick(now + TimeSpan.FromSeconds(6)); + + // Stream has expired and has been removed from pool + Assert.Equal(0, _connection.StreamPool.Count); + + // Removed stream should have been disposed + Assert.True(((Http2OutputProducer)pooledStream.Output)._disposed); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } [Fact]