Fix for #2085 - "The Detaskening" (#2123)

src/Kestrel.Core/Internal/Http/Http1Connection.cs

@@ -413,67 +413,81 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         protected override MessageBody CreateMessageBody()
             => Http1MessageBody.For(_httpVersion, HttpRequestHeaders, this);
 
-        protected override async Task<bool> ParseRequestAsync()
+        protected override void BeginRequestProcessing()
         {
+            // Reset the features and timeout.
             Reset();
             TimeoutControl.SetTimeout(_keepAliveTicks, TimeoutAction.StopProcessingNextRequest);
+        }
 
-            while (_requestProcessingStatus != RequestProcessingStatus.AppStarted)
+        protected override bool BeginRead(out ReadableBufferAwaitable awaitable)
+        {
+            awaitable = Input.ReadAsync();
+            return true;
+        }
+
+        protected override bool TryParseRequest(ReadResult result, out bool endConnection)
+        {
+            var examined = result.Buffer.End;
+            var consumed = result.Buffer.End;
+
+            try
             {
-                var result = await Input.ReadAsync();
+                ParseRequest(result.Buffer, out consumed, out examined);
-
+            }
-                var examined = result.Buffer.End;
+            catch (InvalidOperationException)
-                var consumed = result.Buffer.End;
+            {
-
+                if (_requestProcessingStatus == RequestProcessingStatus.ParsingHeaders)
-                try
                 {
-                    ParseRequest(result.Buffer, out consumed, out examined);
+                    throw BadHttpRequestException.GetException(RequestRejectionReason
-                }
+                        .MalformedRequestInvalidHeaders);
-                catch (InvalidOperationException)
-                {
-                    if (_requestProcessingStatus == RequestProcessingStatus.ParsingHeaders)
-                    {
-                        throw BadHttpRequestException.GetException(RequestRejectionReason
-                            .MalformedRequestInvalidHeaders);
-                    }
-                    throw;
-                }
-                finally
-                {
-                    Input.Advance(consumed, examined);
-                }
-
-                if (result.IsCompleted)
-                {
-                    switch (_requestProcessingStatus)
-                    {
-                        case RequestProcessingStatus.RequestPending:
-                            return false;
-                        case RequestProcessingStatus.ParsingRequestLine:
-                            throw BadHttpRequestException.GetException(
-                                RequestRejectionReason.InvalidRequestLine);
-                        case RequestProcessingStatus.ParsingHeaders:
-                            throw BadHttpRequestException.GetException(
-                                RequestRejectionReason.MalformedRequestInvalidHeaders);
-                    }
-                }
-                else if (!_keepAlive && _requestProcessingStatus == RequestProcessingStatus.RequestPending)
-                {
-                    // Stop the request processing loop if the server is shutting down or there was a keep-alive timeout
-                    // and there is no ongoing request.
-                    return false;
-                }
-                else if (RequestTimedOut)
-                {
-                    // In this case, there is an ongoing request but the start line/header parsing has timed out, so send
-                    // a 408 response.
-                    throw BadHttpRequestException.GetException(RequestRejectionReason.RequestTimeout);
                 }
+                throw;
+            }
+            finally
+            {
+                Input.Advance(consumed, examined);
             }
 
-            EnsureHostHeaderExists();
+            if (result.IsCompleted)
+            {
+                switch (_requestProcessingStatus)
+                {
+                    case RequestProcessingStatus.RequestPending:
+                        endConnection = true;
+                        return true;
+                    case RequestProcessingStatus.ParsingRequestLine:
+                        throw BadHttpRequestException.GetException(
+                            RequestRejectionReason.InvalidRequestLine);
+                    case RequestProcessingStatus.ParsingHeaders:
+                        throw BadHttpRequestException.GetException(
+                            RequestRejectionReason.MalformedRequestInvalidHeaders);
+                }
+            }
+            else if (!_keepAlive && _requestProcessingStatus == RequestProcessingStatus.RequestPending)
+            {
+                // Stop the request processing loop if the server is shutting down or there was a keep-alive timeout
+                // and there is no ongoing request.
+                endConnection = true;
+                return true;
+            }
+            else if (RequestTimedOut)
+            {
+                // In this case, there is an ongoing request but the start line/header parsing has timed out, so send
+                // a 408 response.
+                throw BadHttpRequestException.GetException(RequestRejectionReason.RequestTimeout);
+            }
 
-            return true;
+            endConnection = false;
+            if (_requestProcessingStatus == RequestProcessingStatus.AppStarted)
+            {
+                EnsureHostHeaderExists();
+                return true;
+            }
+            else
+            {
+                return false;
+            }
         }
     }
 }

src/Kestrel.Core/Internal/Http/HttpProtocol.cs

@@ -375,11 +375,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         {
         }
 
+        protected virtual void BeginRequestProcessing()
+        {
+        }
+
+        protected virtual bool BeginRead(out ReadableBufferAwaitable awaitable)
+        {
+            awaitable = default;
+            return false;
+        }
+
         protected abstract string CreateRequestId();
 
         protected abstract MessageBody CreateMessageBody();
 
-        protected abstract Task<bool> ParseRequestAsync();
+        protected abstract bool TryParseRequest(ReadResult result, out bool endConnection);
 
         protected abstract void CreateHttpContext();
 
@@ -436,7 +446,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             {
                 while (_keepAlive)
                 {
-                    if (!await ParseRequestAsync())
+                    BeginRequestProcessing();
+
+                    var result = default(ReadResult);
+                    var endConnection = false;
+                    do
+                    {
+                        if (BeginRead(out var awaitable))
+                        {
+                            result = await awaitable;
+                        }
+                    } while (!TryParseRequest(result, out endConnection));
+
+                    if (endConnection)
                     {
                         return;
                     }
@@ -655,7 +677,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             try
             {
                 var count = onStarting.Count;
-                for(var i = 0; i < count; i++)
+                for (var i = 0; i < count; i++)
                 {
                     var entry = onStarting.Pop();
                     var task = entry.Key.Invoke(entry.Value);

src/Kestrel.Core/Internal/Http2/Http2Stream.cs

@@ -47,8 +47,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         protected override MessageBody CreateMessageBody()
             => Http2MessageBody.For(HttpRequestHeaders, this);
 
-        protected override Task<bool> ParseRequestAsync()
+        protected override bool TryParseRequest(ReadResult result, out bool endConnection)
         {
+            // We don't need any of the parameters because we don't implement BeginRead to actually
+            // do the reading from a pipeline, nor do we use endConnection to report connection-level errors.
+
             Method = RequestHeaders[":method"];
             Scheme = RequestHeaders[":scheme"];
             _httpVersion = Http.HttpVersion.Http2;
@@ -62,7 +65,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
             RequestHeaders["Host"] = RequestHeaders[":authority"];
 
-            return Task.FromResult(true);
+            endConnection = false;
+            return true;
         }
 
         public async Task OnDataAsync(ArraySegment<byte> data, bool endStream)