Merge pull request #2736 from dotnet-maestro-bot/merge/release/2.2-to-master

[automated] Merge branch 'release/2.2' => 'master'

src/Kestrel.Core/Internal/ConnectionDispatcher.cs

@@ -59,6 +59,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
                 {
                     Log.LogCritical(0, ex, $"{nameof(ConnectionDispatcher)}.{nameof(Execute)}() {connectionContext.ConnectionId}");
                 }
+                finally
+                {
+                    // Complete the transport PipeReader and PipeWriter after calling into application code
+                    connectionContext.Transport.Input.Complete();
+                    connectionContext.Transport.Output.Complete();
+                }
             }
         }
 

test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs

@@ -1,7 +1,9 @@
 // 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.Collections.Generic;
+using System.IO.Pipelines;
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal;
@@ -42,5 +44,27 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Verify the scope was disposed after request processing completed
             Assert.True(((TestKestrelTrace)serviceContext.Log).Logger.Scopes.IsEmpty);
         }
+
+        [Fact]
+        public async Task OnConnectionCompletesTransportPipesAfterReturning()
+        {
+            var serviceContext = new TestServiceContext();
+            var tcs = new TaskCompletionSource<object>();
+            var dispatcher = new ConnectionDispatcher(serviceContext, _ => Task.CompletedTask);
+
+            var mockConnection = new Mock<TransportConnection>();
+            var mockPipeReader = new Mock<PipeReader>();
+            var mockPipeWriter = new Mock<PipeWriter>();
+            var mockPipe = new Mock<IDuplexPipe>();
+            mockPipe.Setup(m => m.Input).Returns(mockPipeReader.Object);
+            mockPipe.Setup(m => m.Output).Returns(mockPipeWriter.Object);
+            mockConnection.Setup(m => m.Transport).Returns(mockPipe.Object);
+            var connection = mockConnection.Object;
+
+            await dispatcher.OnConnection(connection);
+
+            mockPipeWriter.Verify(m => m.Complete(It.IsAny<Exception>()), Times.Once());
+            mockPipeReader.Verify(m => m.Complete(It.IsAny<Exception>()), Times.Once());
+        }
     }
 }

test/Kestrel.Core.Tests/Http2ConnectionTests.cs

@@ -131,14 +131,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
         public Http2ConnectionTests()
         {
-            var inlineSchedulingPipeOptions = new PipeOptions(
+            // Always dispatch test code back to the ThreadPool. This prevents deadlocks caused by continuing
+            // Http2Connection.ProcessRequestsAsync() loop with writer locks acquired. Run product code inline to make
+            // it easier to verify request frames are processed correctly immediately after sending the them.
+            var inputPipeOptions = new PipeOptions(
                 pool: _memoryPool,
                 readerScheduler: PipeScheduler.Inline,
+                writerScheduler: PipeScheduler.ThreadPool,
+                useSynchronizationContext: false
+            );
+            var outputPipeOptions = new PipeOptions(
+                pool: _memoryPool,
+                readerScheduler: PipeScheduler.ThreadPool,
                 writerScheduler: PipeScheduler.Inline,
                 useSynchronizationContext: false
             );
 
-            _pair = DuplexPipe.CreateConnectionPair(inlineSchedulingPipeOptions, inlineSchedulingPipeOptions);
+            _pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions);
 
             _noopApplication = context => Task.CompletedTask;
 
@@ -488,12 +497,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 withFlags: (byte)Http2DataFrameFlags.END_STREAM,
                 withStreamId: 3);
 
-            // Ensure that Http2FrameWriter._writeLock isn't acquired when completing the frame processing loop.
-            // Otherwise there's a deadlock where Http2OutputProducer.Abort() being called from the frame processing
-            // loop blocks waiting Http2OutputProducer.Dispose() being called from the stream processing loop to
-            // acquire the _writeLock.
-            await ThreadPoolAwaitable.Instance;
-
             await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
 
             Assert.Equal(stream1DataFrame1.DataPayload, _helloBytes);
@@ -565,12 +568,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 withFlags: (byte)Http2DataFrameFlags.END_STREAM,
                 withStreamId: 1);
 
-            // Ensure that Http2FrameWriter._writeLock isn't acquired when completing the frame processing loop.
-            // Otherwise there's a deadlock where Http2OutputProducer.Abort() being called from the frame processing
-            // loop blocks waiting Http2OutputProducer.Dispose() being called from the stream processing loop to
-            // acquire the _writeLock.
-            await ThreadPoolAwaitable.Instance;
-
             await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
         }
 
@@ -868,12 +865,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 withFlags: (byte)Http2DataFrameFlags.END_STREAM,
                 withStreamId: 1);
 
-            // Ensure that Http2FrameWriter._writeLock isn't acquired when completing the frame processing loop.
-            // Otherwise there's a deadlock where Http2OutputProducer.Abort() being called from the frame processing
-            // loop blocks waiting Http2OutputProducer.Dispose() being called from the stream processing loop to
-            // acquire the _writeLock.
-            await ThreadPoolAwaitable.Instance;
-
             await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
             await WaitForAllStreamsAsync();
         }