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/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx
index 7bbbdfff3a..0cef31d07b 100644
--- a/src/Kestrel.Core/CoreStrings.resx
+++ b/src/Kestrel.Core/CoreStrings.resx
@@ -557,4 +557,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
The request :scheme header '{requestScheme}' does not match the transport scheme '{transportScheme}'.
+
+ An error occured after the response headers were sent, a reset is being sent.
+
\ No newline at end of file
diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs
index 62aa16ef64..6acbc0e378 100644
--- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs
+++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs
@@ -1032,8 +1032,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{
if (HasResponseStarted)
{
- // We can no longer change the response, so we simply close the connection.
- _keepAlive = false;
+ ErrorAfterResponseStarted();
return Task.CompletedTask;
}
@@ -1058,6 +1057,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
return WriteSuffix();
}
+ protected virtual void ErrorAfterResponseStarted()
+ {
+ // We can no longer change the response, so we simply close the connection.
+ _keepAlive = false;
+ }
+
[MethodImpl(MethodImplOptions.NoInlining)]
private async Task ProduceEndAwaited()
{
diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs
index 1267a0fc20..486020224d 100644
--- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs
+++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs
@@ -74,9 +74,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
private RequestHeaderParsingState _requestHeaderParsingState;
private PseudoHeaderFields _parsedPseudoHeaderFields;
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();
@@ -97,20 +97,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)
@@ -127,12 +167,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;
@@ -197,11 +237,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();
@@ -210,7 +259,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
stream.Abort(connectionError);
}
- await _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, errorCode);
_frameWriter.Complete();
}
catch
@@ -244,7 +292,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;
@@ -623,7 +671,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;
}
@@ -723,12 +772,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)
@@ -775,11 +832,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;
@@ -815,7 +867,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)
@@ -943,6 +1007,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/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs
index a0828e7ed2..6319de96b9 100644
--- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs
+++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs
@@ -333,10 +333,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
AbortCore(abortReason);
}
+ protected override void ErrorAfterResponseStarted()
+ {
+ // We can no longer change the response, send a Reset instead.
+ base.ErrorAfterResponseStarted();
+ var abortReason = new ConnectionAbortedException(CoreStrings.Http2StreamErrorAfterHeaders);
+ ResetAndAbort(abortReason, Http2ErrorCode.INTERNAL_ERROR);
+ }
+
protected override void ApplicationAbort()
{
var abortReason = new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication);
- ResetAndAbort(abortReason, Http2ErrorCode.CANCEL);
+ ResetAndAbort(abortReason, Http2ErrorCode.INTERNAL_ERROR);
}
private void ResetAndAbort(ConnectionAbortedException abortReason, Http2ErrorCode error)
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/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs
index 09e593c57b..fddd203d4a 100644
--- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs
+++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs
@@ -2058,6 +2058,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
internal static string FormatHttp2StreamErrorSchemeMismatch(object requestScheme, object transportScheme)
=> string.Format(CultureInfo.CurrentCulture, GetString("Http2StreamErrorSchemeMismatch", "requestScheme", "transportScheme"), requestScheme, transportScheme);
+ ///
+ /// An error occured after the response headers were sent, a reset is being sent.
+ ///
+ internal static string Http2StreamErrorAfterHeaders
+ {
+ get => GetString("Http2StreamErrorAfterHeaders");
+ }
+
+ ///
+ /// An error occured after the response headers were sent, a reset is being sent.
+ ///
+ internal static string FormatHttp2StreamErrorAfterHeaders()
+ => GetString("Http2StreamErrorAfterHeaders");
+
private static string GetString(string name, params string[] formatterNames)
{
var value = _resourceManager.GetString(name);
diff --git a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs
index 1a6c8b16d8..17eba8078f 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