Only reset Http2Stream on stream completion (#19431)

This commit is contained in:
James Newton-King 2020-03-04 08:37:31 +13:00 committed by GitHub
parent 3df570fac6
commit 065f0d001c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 71 additions and 9 deletions

View File

@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
public Http1Connection(HttpConnectionContext context) public Http1Connection(HttpConnectionContext context)
{ {
Initialize(context); Initialize(context, reset: true);
_context = context; _context = context;
_parser = ServiceContext.HttpParser; _parser = ServiceContext.HttpParser;

View File

@ -76,13 +76,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private Stream _requestStreamInternal; private Stream _requestStreamInternal;
private Stream _responseStreamInternal; private Stream _responseStreamInternal;
public void Initialize(HttpConnectionContext context) public void Initialize(HttpConnectionContext context, bool reset)
{ {
_context = context; _context = context;
ServerOptions = ServiceContext.ServerOptions; ServerOptions = ServiceContext.ServerOptions;
Reset(); if (reset)
{
Reset();
}
HttpResponseControl = this; HttpResponseControl = this;
} }

View File

@ -33,9 +33,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
private StreamCompletionFlags _completionState; private StreamCompletionFlags _completionState;
private readonly object _completionLock = new object(); private readonly object _completionLock = new object();
public void Initialize(Http2StreamContext context) /// <summary>
/// Initialize the stream with the specified context.
/// </summary>
/// <param name="context">The context.</param>
/// <param name="reset">
/// A value indicating whether to reset the protocol instance.
/// We want to reset when the stream is created, and when the stream is returned to the pool.
/// The stream shouldn't be reset when fetched from the pool.
/// </param>
public void InitializePooled(Http2StreamContext context, bool reset)
{ {
base.Initialize(context); base.Initialize(context, reset);
_decrementCalled = false; _decrementCalled = false;
_completionState = StreamCompletionFlags.None; _completionState = StreamCompletionFlags.None;
@ -71,7 +80,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
public void InitializeWithExistingContext(int streamId) public void InitializeWithExistingContext(int streamId)
{ {
_context.StreamId = streamId; _context.StreamId = streamId;
Initialize(_context);
// The stream was reset when it was completed.
// We don't want to reset it twice because reused headers will be discarded.
InitializePooled(_context, reset: false);
} }
public int StreamId => _context.StreamId; public int StreamId => _context.StreamId;

View File

@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
public Http2Stream(IHttpApplication<TContext> application, Http2StreamContext context) public Http2Stream(IHttpApplication<TContext> application, Http2StreamContext context)
{ {
Initialize(context); InitializePooled(context, reset: true);
_application = application; _application = application;
} }

View File

@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
public Http3Stream(Http3Connection http3Connection, Http3StreamContext context) public Http3Stream(Http3Connection http3Connection, Http3StreamContext context)
{ {
Initialize(context); Initialize(context, reset: true);
InputRemaining = null; InputRemaining = null;

View File

@ -251,7 +251,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
{ {
public TestHttp2Stream(Http2StreamContext context) public TestHttp2Stream(Http2StreamContext context)
{ {
Initialize(context); InitializePooled(context, reset: true);
} }
public override void Execute() public override void Execute()

View File

@ -25,6 +25,53 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
{ {
public class Http2ConnectionTests : Http2TestBase public class Http2ConnectionTests : Http2TestBase
{ {
[Fact]
public async Task RequestHeaderStringReuse_MultipleStreams_KnownHeaderReused()
{
IEnumerable<KeyValuePair<string, string>> requestHeaders = new[]
{
new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
new KeyValuePair<string, string>(HeaderNames.Path, "/hello"),
new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
new KeyValuePair<string, string>(HeaderNames.ContentType, "application/json")
};
await InitializeConnectionAsync(_readHeadersApplication);
await StartStreamAsync(1, requestHeaders, endStream: true);
await ExpectAsync(Http2FrameType.HEADERS,
withLength: 55,
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
withStreamId: 1);
var contentType1 = _receivedHeaders["Content-Type"];
// 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);
await StartStreamAsync(3, requestHeaders, endStream: true);
await ExpectAsync(Http2FrameType.HEADERS,
withLength: 55,
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
withStreamId: 3);
var contentType2 = _receivedHeaders["Content-Type"];
Assert.Same(contentType1, contentType2);
await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
}
[Fact] [Fact]
public async Task StreamPool_SingleStream_ReturnedToPool() public async Task StreamPool_SingleStream_ReturnedToPool()
{ {