From 8a74cf3ed54c5af3415beadcc92ce857b2b8dcba Mon Sep 17 00:00:00 2001 From: John Luo Date: Mon, 30 Jul 2018 11:05:24 -0700 Subject: [PATCH] Gracefully shutdown HTTP/2 connections on server and client initiated shutdown --- .../Kestrel.Performance/Mocks/MockTrace.cs | 2 + .../Internal/Http2/Http2Connection.cs | 123 +++++++-- .../Internal/Http2/Http2ConnectionState.cs | 12 + .../Internal/Http2/Http2Frame.GoAway.cs | 4 +- .../Internal/Http2/Http2Frame.RstStream.cs | 2 +- .../Internal/Infrastructure/IKestrelTrace.cs | 4 + .../Internal/Infrastructure/KestrelTrace.cs | 18 ++ .../Http2ConnectionTests.cs | 240 +++++++++++++++--- .../Http2/ShutdownTests.cs | 123 +++++++++ .../TestHelpers/TestServer.cs | 10 +- test/shared/CompositeKestrelTrace.cs | 14 +- test/shared/TestServiceContext.cs | 3 - 12 files changed, 485 insertions(+), 70 deletions(-) create mode 100644 src/Kestrel.Core/Internal/Http2/Http2ConnectionState.cs create mode 100644 test/Kestrel.FunctionalTests/Http2/ShutdownTests.cs diff --git a/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs b/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs index b14fd2e14f..cf95b491c9 100644 --- a/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs +++ b/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs @@ -50,5 +50,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public void Http2StreamError(string connectionId, Http2StreamErrorException ex) { } public void HPackDecodingError(string connectionId, int streamId, HPackDecodingException ex) { } public void Http2StreamResetAbort(string traceIdentifier, Http2ErrorCode error, ConnectionAbortedException abortReason) { } + public void Http2ConnectionClosing(string connectionId) { } + public void Http2ConnectionClosed(string connectionId, int highestOpenedStreamId) { } } } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 487b8fae27..5832379e26 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -75,9 +75,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private PseudoHeaderFields _parsedPseudoHeaderFields; private Http2HeadersFrameFlags _headerFlags; private bool _isMethodConnect; + private readonly object _stateLock = new object(); private int _highestOpenedStreamId; - - private bool _stopping; + private Http2ConnectionState _state = Http2ConnectionState.Open; private readonly ConcurrentDictionary _streams = new ConcurrentDictionary(); @@ -98,20 +98,60 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public void OnInputOrOutputCompleted() { - _stopping = true; + lock (_stateLock) + { + if (_state != Http2ConnectionState.Closed) + { + _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR); + UpdateState(Http2ConnectionState.Closed); + } + } + _frameWriter.Complete(); } public void Abort(ConnectionAbortedException ex) { - _stopping = true; + lock (_stateLock) + { + if (_state != Http2ConnectionState.Closed) + { + _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.INTERNAL_ERROR); + UpdateState(Http2ConnectionState.Closed); + } + } + _frameWriter.Abort(ex); } public void StopProcessingNextRequest() + => StopProcessingNextRequest(true); + + public void StopProcessingNextRequest(bool sendGracefulGoAway = false) { - _stopping = true; - Input.CancelPendingRead(); + lock (_stateLock) + { + if (_state == Http2ConnectionState.Open) + { + if (_streams.IsEmpty) + { + _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR); + UpdateState(Http2ConnectionState.Closed); + + // Wake up request processing loop so the connection can complete if there are no pending requests + Input.CancelPendingRead(); + } + else + { + if (sendGracefulGoAway) + { + _frameWriter.WriteGoAwayAsync(Int32.MaxValue, Http2ErrorCode.NO_ERROR); + } + + UpdateState(Http2ConnectionState.Closing); + } + } + } } public async Task ProcessRequestsAsync(IHttpApplication application) @@ -128,12 +168,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return; } - if (!_stopping) + if (_state != Http2ConnectionState.Closed) { await _frameWriter.WriteSettingsAsync(_serverSettings); } - while (!_stopping) + while (_state != Http2ConnectionState.Closed) { var result = await Input.ReadAsync(); var readableBuffer = result.Buffer; @@ -198,11 +238,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } finally { - var connectionError = error as ConnectionAbortedException + var connectionError = error as ConnectionAbortedException ?? new ConnectionAbortedException(CoreStrings.Http2ConnectionFaulted, error); try { + lock (_stateLock) + { + if (_state != Http2ConnectionState.Closed) + { + _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, errorCode); + UpdateState(Http2ConnectionState.Closed); + } + } + // Ensure aborting each stream doesn't result in unnecessary WINDOW_UPDATE frames being sent. _inputFlowControl.StopWindowUpdates(); @@ -211,7 +260,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 stream.Abort(connectionError); } - await _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, errorCode); _frameWriter.Complete(); } catch @@ -245,7 +293,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private async Task TryReadPrefaceAsync() { - while (!_stopping) + while (_state != Http2ConnectionState.Closed) { var result = await Input.ReadAsync(); var readableBuffer = result.Buffer; @@ -625,7 +673,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamIdNotZero(_incomingFrame.Type), Http2ErrorCode.PROTOCOL_ERROR); } - StopProcessingNextRequest(); + StopProcessingNextRequest(sendGracefulGoAway: false); + return Task.CompletedTask; } @@ -725,12 +774,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { try { - _hpackDecoder.Decode(payload, endHeaders, handler: this); - - if (endHeaders) + lock (_stateLock) { - StartStream(application); - ResetRequestHeaderParsingState(); + _highestOpenedStreamId = _currentHeadersStream.StreamId; + _hpackDecoder.Decode(payload, endHeaders, handler: this); + + if (endHeaders) + { + if (_state != Http2ConnectionState.Closed) + { + StartStream(application); + } + + ResetRequestHeaderParsingState(); + } } } catch (Http2StreamErrorException) @@ -786,11 +843,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private void ResetRequestHeaderParsingState() { - if (_requestHeaderParsingState != RequestHeaderParsingState.Trailers) - { - _highestOpenedStreamId = _currentHeadersStream.StreamId; - } - _currentHeadersStream = null; _requestHeaderParsingState = RequestHeaderParsingState.Ready; _parsedPseudoHeaderFields = PseudoHeaderFields.None; @@ -827,7 +879,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 void IHttp2StreamLifetimeHandler.OnStreamCompleted(int streamId) { - _streams.TryRemove(streamId, out _); + lock (_stateLock) + { + _streams.TryRemove(streamId, out _); + + if (_state == Http2ConnectionState.Closing && _streams.IsEmpty) + { + _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR); + UpdateState(Http2ConnectionState.Closed); + + // Wake up request processing loop so the connection can complete if there are no pending requests + Input.CancelPendingRead(); + } + } } public void OnHeader(Span name, Span value) @@ -955,6 +1019,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return name.SequenceEqual(_connectionBytes) || (name.SequenceEqual(_teBytes) && !value.SequenceEqual(_trailersBytes)); } + private void UpdateState(Http2ConnectionState state) + { + _state = state; + if (state == Http2ConnectionState.Closing) + { + Log.Http2ConnectionClosing(_context.ConnectionId); + } + else if (state == Http2ConnectionState.Closed) + { + Log.Http2ConnectionClosed(_context.ConnectionId, _highestOpenedStreamId); + } + } + void ITimeoutControl.SetTimeout(long ticks, TimeoutAction timeoutAction) { } diff --git a/src/Kestrel.Core/Internal/Http2/Http2ConnectionState.cs b/src/Kestrel.Core/Internal/Http2/Http2ConnectionState.cs new file mode 100644 index 0000000000..cb1518807d --- /dev/null +++ b/src/Kestrel.Core/Internal/Http2/Http2ConnectionState.cs @@ -0,0 +1,12 @@ +// 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. + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 +{ + public enum Http2ConnectionState + { + Open = 0, + Closing, + Closed + } +} diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.GoAway.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.GoAway.cs index 3e430de8b0..e3bec2afa2 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.GoAway.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.GoAway.cs @@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { public int GoAwayLastStreamId { - get => (Payload[0] << 24) | (Payload[1] << 16) | (Payload[2] << 16) | Payload[3]; + get => (Payload[0] << 24) | (Payload[1] << 16) | (Payload[2] << 8) | Payload[3]; set { Payload[0] = (byte)((value >> 24) & 0xff); @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public Http2ErrorCode GoAwayErrorCode { - get => (Http2ErrorCode)((Payload[4] << 24) | (Payload[5] << 16) | (Payload[6] << 16) | Payload[7]); + get => (Http2ErrorCode)((Payload[4] << 24) | (Payload[5] << 16) | (Payload[6] << 8) | Payload[7]); set { Payload[4] = (byte)(((uint)value >> 24) & 0xff); diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.RstStream.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.RstStream.cs index 8a0bcdfd6c..612d778d14 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.RstStream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.RstStream.cs @@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { public Http2ErrorCode RstStreamErrorCode { - get => (Http2ErrorCode)((Payload[0] << 24) | (Payload[1] << 16) | (Payload[2] << 16) | Payload[3]); + get => (Http2ErrorCode)((Payload[0] << 24) | (Payload[1] << 16) | (Payload[2] << 8) | Payload[3]); set { Payload[0] = (byte)(((uint)value >> 24) & 0xff); diff --git a/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs b/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs index d29b1b43d8..fd9b2b78a5 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs @@ -57,6 +57,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure void Http2ConnectionError(string connectionId, Http2ConnectionErrorException ex); + void Http2ConnectionClosing(string connectionId); + + void Http2ConnectionClosed(string connectionId, int highestOpenedStreamId); + void Http2StreamError(string connectionId, Http2StreamErrorException ex); void Http2StreamResetAbort(string traceIdentifier, Http2ErrorCode error, ConnectionAbortedException abortReason); diff --git a/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs b/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs index 8f4a22539e..8a489eb939 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs @@ -91,6 +91,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal LoggerMessage.Define(LogLevel.Debug, new EventId(35, nameof(Http2StreamResetAbort)), @"Trace id ""{TraceIdentifier}"": HTTP/2 stream error ""{error}"". A Reset is being sent to the stream."); + private static readonly Action _http2ConnectionClosing = + LoggerMessage.Define(LogLevel.Debug, new EventId(36, nameof(Http2ConnectionClosing)), + @"Connection id ""{ConnectionId}"" is closing."); + + private static readonly Action _http2ConnectionClosed = + LoggerMessage.Define(LogLevel.Debug, new EventId(36, nameof(Http2ConnectionClosed)), + @"Connection id ""{ConnectionId}"" is closed. The last processed stream ID was {HighestOpenedStreamId}."); + protected readonly ILogger _logger; public KestrelTrace(ILogger logger) @@ -213,6 +221,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _http2ConnectionError(_logger, connectionId, ex); } + public virtual void Http2ConnectionClosing(string connectionId) + { + _http2ConnectionClosing(_logger, connectionId, null); + } + + public virtual void Http2ConnectionClosed(string connectionId, int highestOpenedStreamId) + { + _http2ConnectionClosed(_logger, connectionId, highestOpenedStreamId, null); + } + public virtual void Http2StreamError(string connectionId, Http2StreamErrorException ex) { _http2StreamError(_logger, connectionId, ex); diff --git a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs index fe146599f0..497662a2e7 100644 --- a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs +++ b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs @@ -11,8 +11,9 @@ using System.Linq; using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; @@ -21,8 +22,8 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Net.Http.Headers; +using Moq; using Xunit; -using Microsoft.AspNetCore.Http.Features; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { @@ -110,6 +111,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests private readonly Dictionary _decodedHeaders = new Dictionary(StringComparer.OrdinalIgnoreCase); private readonly HashSet _abortedStreamIds = new HashSet(); private readonly object _abortedStreamIdsLock = new object(); + private readonly TaskCompletionSource _closingStateReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly TaskCompletionSource _closedStateReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); private readonly RequestDelegate _noopApplication; private readonly RequestDelegate _readHeadersApplication; @@ -305,12 +308,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions); + var mockKestrelTrace = new Mock(_logger) + { + CallBase = true + }; + mockKestrelTrace + .Setup(m => m.Http2ConnectionClosing(It.IsAny())) + .Callback(() => _closingStateReached.SetResult(null)); + mockKestrelTrace + .Setup(m => m.Http2ConnectionClosed(It.IsAny(), It.IsAny())) + .Callback(() => _closedStateReached.SetResult(null)); + _connectionContext = new Http2ConnectionContext { ConnectionFeatures = new FeatureCollection(), ServiceContext = new TestServiceContext() { - Log = new TestKestrelTrace(_logger) + Log = mockKestrelTrace.Object }, MemoryPool = _memoryPool, Application = _pair.Application, @@ -946,7 +960,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.DATA, streamId: 1, headersStreamId: 1)); } @@ -1585,7 +1599,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.HEADERS, streamId: 3, headersStreamId: 1)); } @@ -1613,7 +1627,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.COMPRESSION_ERROR, expectedErrorMessage: CoreStrings.HPackErrorIncompleteHeaderBlock); } @@ -1869,7 +1883,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.PRIORITY, streamId: 1, headersStreamId: 1)); } @@ -2190,7 +2204,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.RST_STREAM, streamId: 1, headersStreamId: 1)); } @@ -2262,7 +2276,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.SETTINGS, streamId: 0, headersStreamId: 1)); } @@ -2369,7 +2383,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.PING, streamId: 0, headersStreamId: 1)); } @@ -2417,23 +2431,47 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Fact] - public async Task GOAWAY_Received_AbortsAllStreams() + public async Task GOAWAY_Received_SetsConnectionStateToClosingAndWaitForAllStreamsToComplete() { - await InitializeConnectionAsync(_waitForAbortApplication); + await InitializeConnectionAsync(_echoApplication); // Start some streams - await StartStreamAsync(1, _browserRequestHeaders, endStream: true); - await StartStreamAsync(3, _browserRequestHeaders, endStream: true); - await StartStreamAsync(5, _browserRequestHeaders, endStream: true); + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + await StartStreamAsync(3, _browserRequestHeaders, endStream: false); await SendGoAwayAsync(); - await WaitForConnectionStopAsync(expectedLastStreamId: 5, ignoreNonGoAwayFrames: true); + await _closingStateReached.Task.DefaultTimeout(); - await WaitForAllStreamsAsync(); - Assert.Contains(1, _abortedStreamIds); - Assert.Contains(3, _abortedStreamIds); - Assert.Contains(5, _abortedStreamIds); + await SendDataAsync(1, _helloBytes, true); + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 5, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + await SendDataAsync(3, _helloBytes, true); + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + await ExpectAsync(Http2FrameType.DATA, + withLength: 5, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 3); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 3); + + await WaitForConnectionStopAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false); + await _closedStateReached.Task.DefaultTimeout(); } [Fact] @@ -2526,7 +2564,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 5); - await SendGoAwayAsync(); + // Close all pipes and wait for response to drain + _pair.Application.Output.Complete(); + _pair.Transport.Input.Complete(); + _pair.Transport.Output.Complete(); await WaitForConnectionStopAsync(expectedLastStreamId: 5, ignoreNonGoAwayFrames: false); @@ -2600,7 +2641,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await VerifyStreamBackpressure(3); await VerifyStreamBackpressure(5); - await SendGoAwayAsync(); + // Close all pipes and wait for response to drain + _pair.Application.Output.Complete(); + _pair.Transport.Input.Complete(); + _pair.Transport.Output.Complete(); await WaitForConnectionStopAsync(expectedLastStreamId: 5, ignoreNonGoAwayFrames: false); @@ -2634,7 +2678,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.GOAWAY, streamId: 0, headersStreamId: 1)); } @@ -2663,7 +2707,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.WINDOW_UPDATE, streamId: 1, headersStreamId: 1)); } @@ -2784,7 +2828,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { try { - // Flush the headers so expectingDataSem is released. + // Flush the headers so expectingDataSem is released. await context.Response.Body.FlushAsync(); for (var i = 0; i < expectedFullFrameCountBeforeBackpressure; i++) @@ -3045,7 +3089,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.CONTINUATION, streamId: 3, headersStreamId: 1)); } @@ -3060,7 +3104,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.COMPRESSION_ERROR, expectedErrorMessage: CoreStrings.HPackErrorIncompleteHeaderBlock); } @@ -3197,7 +3241,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, + expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(frameType: 42, streamId: 1, headersStreamId: 1)); } @@ -3253,6 +3297,129 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.DoesNotContain(_logger.Messages, m => m.Exception is ConnectionResetException); } + [Fact] + public async Task OnInputOrOutputCompletedSendsFinalGOAWAY() + { + await InitializeConnectionAsync(_noopApplication); + + _connection.OnInputOrOutputCompleted(); + await _closedStateReached.Task.DefaultTimeout(); + + VerifyGoAway(await ReceiveFrameAsync(), 0, Http2ErrorCode.NO_ERROR); + } + + [Fact] + public async Task AbortSendsFinalGOAWAY() + { + await InitializeConnectionAsync(_noopApplication); + + _connection.Abort(new ConnectionAbortedException()); + await _closedStateReached.Task.DefaultTimeout(); + + VerifyGoAway(await ReceiveFrameAsync(), 0, Http2ErrorCode.INTERNAL_ERROR); + } + + [Fact] + public async Task CompletionSendsFinalGOAWAY() + { + await InitializeConnectionAsync(_noopApplication); + + // Completes ProcessRequestsAsync + _pair.Application.Output.Complete(); + await _closedStateReached.Task.DefaultTimeout(); + + VerifyGoAway(await ReceiveFrameAsync(), 0, Http2ErrorCode.NO_ERROR); + } + + [Fact] + public async Task StopProcessingNextRequestSendsGracefulGOAWAYThenFinalGOAWAYWhenAllStreamsComplete() + { + await InitializeConnectionAsync(_echoApplication); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + + _connection.StopProcessingNextRequest(); + await _closingStateReached.Task.DefaultTimeout(); + + VerifyGoAway(await ReceiveFrameAsync(), Int32.MaxValue, Http2ErrorCode.NO_ERROR); + + await SendDataAsync(1, _helloBytes, true); + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 5, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + await _closedStateReached.Task.DefaultTimeout(); + VerifyGoAway(await ReceiveFrameAsync(), 1, Http2ErrorCode.NO_ERROR); + } + + [Fact] + public async Task AcceptNewStreamsDuringClosingConnection() + { + await InitializeConnectionAsync(_echoApplication); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + + _connection.StopProcessingNextRequest(); + VerifyGoAway(await ReceiveFrameAsync(), Int32.MaxValue, Http2ErrorCode.NO_ERROR); + + await _closingStateReached.Task.DefaultTimeout(); + + await StartStreamAsync(3, _browserRequestHeaders, endStream: false); + + await SendDataAsync(1, _helloBytes, true); + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 5, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + await SendDataAsync(3, _helloBytes, true); + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + await ExpectAsync(Http2FrameType.DATA, + withLength: 5, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 3); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 3); + + await WaitForConnectionStopAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false); + } + + [Fact] + public async Task IgnoreNewStreamsDuringClosedConnection() + { + await InitializeConnectionAsync(_echoApplication); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + + _connection.OnInputOrOutputCompleted(); + await _closedStateReached.Task.DefaultTimeout(); + + await StartStreamAsync(3, _browserRequestHeaders, endStream: false); + + await WaitForConnectionStopAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + private async Task InitializeConnectionAsync(RequestDelegate application) { _connectionTask = _connection.ProcessRequestsAsync(new DummyApplication(application)); @@ -3751,6 +3918,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests return WaitForConnectionErrorAsync(ignoreNonGoAwayFrames, expectedLastStreamId, Http2ErrorCode.NO_ERROR, expectedErrorMessage: null); } + private void VerifyGoAway(Http2Frame frame, int expectedLastStreamId, Http2ErrorCode expectedErrorCode) + { + Assert.Equal(Http2FrameType.GOAWAY, frame.Type); + Assert.Equal(8, frame.Length); + Assert.Equal(0, frame.Flags); + Assert.Equal(0, frame.StreamId); + Assert.Equal(expectedLastStreamId, frame.GoAwayLastStreamId); + Assert.Equal(expectedErrorCode, frame.GoAwayErrorCode); + } + private async Task WaitForConnectionErrorAsync(bool ignoreNonGoAwayFrames, int expectedLastStreamId, Http2ErrorCode expectedErrorCode, string expectedErrorMessage) where TException : Exception { @@ -3764,12 +3941,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } } - Assert.Equal(Http2FrameType.GOAWAY, frame.Type); - Assert.Equal(8, frame.Length); - Assert.Equal(0, frame.Flags); - Assert.Equal(0, frame.StreamId); - Assert.Equal(expectedLastStreamId, frame.GoAwayLastStreamId); - Assert.Equal(expectedErrorCode, frame.GoAwayErrorCode); + VerifyGoAway(frame, expectedLastStreamId, expectedErrorCode); if (expectedErrorMessage != null) { diff --git a/test/Kestrel.FunctionalTests/Http2/ShutdownTests.cs b/test/Kestrel.FunctionalTests/Http2/ShutdownTests.cs new file mode 100644 index 0000000000..b03bc4fb64 --- /dev/null +++ b/test/Kestrel.FunctionalTests/Http2/ShutdownTests.cs @@ -0,0 +1,123 @@ +// 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. + +#if NETCOREAPP2_2 + +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Runtime.InteropServices; +using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; +using Microsoft.AspNetCore.Testing; +using Microsoft.AspNetCore.Testing.xunit; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 +{ + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "Missing SslStream ALPN support: https://github.com/dotnet/corefx/issues/30492")] + [OSSkipCondition(OperatingSystems.Linux, SkipReason = "Curl requires a custom install to support HTTP/2, see https://askubuntu.com/questions/884899/how-do-i-install-curl-with-http2-support")] + [MinimumOSVersion(OperatingSystems.Windows, WindowsVersions.Win10)] + public class ShutdownTests : TestApplicationErrorLoggerLoggedTest + { + private static X509Certificate2 _x509Certificate2 = TestResources.GetTestCertificate(); + + private HttpClient Client { get; set; } + private List ReceivedFrames { get; } = new List(); + + public ShutdownTests() + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // We don't want the default SocketsHttpHandler, it doesn't support HTTP/2 yet. + Client = new HttpClient(new WinHttpHandler() + { + ServerCertificateValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator + }); + } + } + + [ConditionalFact] + public async Task GracefulShutdownWaitsForRequestsToFinish() + { + var requestStarted = new TaskCompletionSource(); + var requestUnblocked = new TaskCompletionSource(); + using (var server = new TestServer(async context => + { + requestStarted.SetResult(null); + await requestUnblocked.Task.DefaultTimeout(); + await context.Response.WriteAsync("hello world " + context.Request.Protocol); + }, + new TestServiceContext(LoggerFactory), + kestrelOptions => + { + kestrelOptions.Listen(IPAddress.Loopback, 0, listenOptions => + { + listenOptions.Protocols = HttpProtocols.Http2; + listenOptions.UseHttps(_x509Certificate2); + }); + })) + { + var requestTask = Client.GetStringAsync($"https://localhost:{server.Port}/"); + Assert.False(requestTask.IsCompleted); + + await requestStarted.Task.DefaultTimeout(); + + var stopTask = server.StopAsync(); + + // Unblock the request + requestUnblocked.SetResult(null); + + Assert.Equal("hello world HTTP/2", await requestTask); + await stopTask.DefaultTimeout(); + } + + Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Message.Contains("Request finished in")); + Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Message.Contains("is closing.")); + Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Message.Contains("is closed. The last processed stream ID was 1.")); + } + + [ConditionalFact] + public async Task GracefulTurnsAbortiveIfRequestsDoNotFinish() + { + var requestStarted = new TaskCompletionSource(); + var requestUnblocked = new TaskCompletionSource(); + // Abortive shutdown leaves one request hanging + using (var server = new TestServer(TransportSelector.GetWebHostBuilder(new DiagnosticMemoryPoolFactory(allowLateReturn: true).Create), async context => + { + requestStarted.SetResult(null); + await requestUnblocked.Task.DefaultTimeout(); + await context.Response.WriteAsync("hello world " + context.Request.Protocol); + }, new TestServiceContext(LoggerFactory), + kestrelOptions => + { + kestrelOptions.Listen(IPAddress.Loopback, 0, listenOptions => + { + listenOptions.Protocols = HttpProtocols.Http2; + listenOptions.UseHttps(_x509Certificate2); + }); + }, + _ => { })) + { + var requestTask = Client.GetStringAsync($"https://localhost:{server.Port}/"); + Assert.False(requestTask.IsCompleted); + await requestStarted.Task.DefaultTimeout(); + + await server.StopAsync().DefaultTimeout(); + } + + Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Message.Contains("is closing.")); + Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Message.Contains("is closed. The last processed stream ID was 1.")); + Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Message.Contains("Some connections failed to close gracefully during server shutdown.")); + Assert.DoesNotContain(TestApplicationErrorLogger.Messages, m => m.Message.Contains("Request finished in")); + } + } +} +#elif NET461 // No ALPN support +#else +#error TFMs need updating +#endif \ No newline at end of file diff --git a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs index 9b290fdc37..8017e4313e 100644 --- a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs +++ b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs @@ -12,7 +12,6 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; @@ -45,20 +44,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } public TestServer(RequestDelegate app, TestServiceContext context, ListenOptions listenOptions, Action configureServices) - : this(app, context, options => options.ListenOptions.Add(listenOptions), configureServices) + : this(TransportSelector.GetWebHostBuilder(), app, context, options => options.ListenOptions.Add(listenOptions), configureServices) { } public TestServer(RequestDelegate app, TestServiceContext context, Action configureKestrel) - : this(app, context, configureKestrel, _ => { }) + : this(TransportSelector.GetWebHostBuilder(), app, context, configureKestrel, _ => { }) { } - public TestServer(RequestDelegate app, TestServiceContext context, Action configureKestrel, Action configureServices) + public TestServer(IWebHostBuilder builder, RequestDelegate app, TestServiceContext context, Action configureKestrel, Action configureServices) { _app = app; Context = context; - _host = TransportSelector.GetWebHostBuilder() + _host = builder .UseKestrel(options => { configureKestrel(options); @@ -82,6 +81,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests configureServices(services); }) .UseSetting(WebHostDefaults.ApplicationKey, typeof(TestServer).GetTypeInfo().Assembly.FullName) + .UseSetting(WebHostDefaults.ShutdownTimeoutKey, "1") .Build(); _host.Start(); diff --git a/test/shared/CompositeKestrelTrace.cs b/test/shared/CompositeKestrelTrace.cs index 4e3a1ca13d..ff5801f628 100644 --- a/test/shared/CompositeKestrelTrace.cs +++ b/test/shared/CompositeKestrelTrace.cs @@ -2,11 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.IO.Pipelines; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -196,5 +194,17 @@ namespace Microsoft.AspNetCore.Testing _trace1.Http2StreamResetAbort(traceIdentifier, error, abortReason); _trace2.Http2StreamResetAbort(traceIdentifier, error, abortReason); } + + public void Http2ConnectionClosing(string connectionId) + { + _trace1.Http2ConnectionClosing(connectionId); + _trace2.Http2ConnectionClosing(connectionId); + } + + public void Http2ConnectionClosed(string connectionId, int highestOpenedStreamId) + { + _trace1.Http2ConnectionClosed(connectionId, highestOpenedStreamId); + _trace2.Http2ConnectionClosed(connectionId, highestOpenedStreamId); + } } } diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index d2d4252dc6..1c5e853d08 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -1,13 +1,10 @@ // 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; using System.IO.Pipelines; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.Extensions.Logging;