From e1de6f5d48083373243ef0115005fde9eda2e4ad Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 17 Mar 2021 19:38:12 +1300 Subject: [PATCH 1/2] Dispose streams that don't fit in pool --- .../Kestrel/Core/src/Internal/Http2/Http2Connection.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 6562256fbde4..fe60945fec0f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -668,7 +668,7 @@ private Http2StreamContext CreateHttp2StreamContext() return streamContext; } - private void ReturnStream(Http2Stream stream) + private bool ReturnStream(Http2Stream stream) { // We're conservative about what streams we can reuse. // If there is a chance the stream is still in use then don't attempt to reuse it. @@ -680,7 +680,10 @@ private void ReturnStream(Http2Stream stream) stream.DrainExpirationTicks = SystemClock.UtcNowTicks + StreamPoolExpiryTicks; StreamPool.Push(stream); + return true; } + + return false; } private Task ProcessPriorityFrameAsync() @@ -1181,12 +1184,13 @@ private void UpdateCompletedStreams() private void RemoveStream(Http2Stream stream) { _streams.Remove(stream.StreamId); - if (stream.CanReuse) + if (stream.CanReuse && ReturnStream(stream)) { - ReturnStream(stream); + // Completed stream is placed into pool. } else { + // Stream didn't complete gracefully or pool is full. stream.Dispose(); } } From f20ce1dd1473d427d1577a36886339dc60374fce Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 18 Mar 2021 08:08:19 +1300 Subject: [PATCH 2/2] PR feedback --- .../src/Internal/Http2/Http2Connection.cs | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index fe60945fec0f..3d1a8628c5a5 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -668,24 +668,6 @@ private Http2StreamContext CreateHttp2StreamContext() return streamContext; } - private bool ReturnStream(Http2Stream stream) - { - // We're conservative about what streams we can reuse. - // If there is a chance the stream is still in use then don't attempt to reuse it. - Debug.Assert(stream.CanReuse); - - if (StreamPool.Count < MaxStreamPoolSize) - { - // This property is used to remove unused streams from the pool - stream.DrainExpirationTicks = SystemClock.UtcNowTicks + StreamPoolExpiryTicks; - - StreamPool.Push(stream); - return true; - } - - return false; - } - private Task ProcessPriorityFrameAsync() { if (_currentHeadersStream != null) @@ -1184,9 +1166,15 @@ private void UpdateCompletedStreams() private void RemoveStream(Http2Stream stream) { _streams.Remove(stream.StreamId); - if (stream.CanReuse && ReturnStream(stream)) + + if (stream.CanReuse && StreamPool.Count < MaxStreamPoolSize) { - // Completed stream is placed into pool. + // Pool and reuse the stream if it finished in a graceful state and there is space in the pool. + + // This property is used to remove unused streams from the pool + stream.DrainExpirationTicks = SystemClock.UtcNowTicks + StreamPoolExpiryTicks; + + StreamPool.Push(stream); } else {