From 50b396cec6337956098725f7324711d1b9994370 Mon Sep 17 00:00:00 2001
From: David Fowler <davidfowl@gmail.com>
Date: Mon, 29 Jan 2018 12:41:45 -0800
Subject: [PATCH] Fixed race in sockets transport (#2279)

- Based on the changes you made earlier (f4d27e6), we trigger OnConnectionClosed before the socket is disposed in the SocketTransport. This moves the call to Output.Complete to happen after and thus fixes the race.
---
 .../Internal/SocketConnection.cs                 | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs
index ff7c678cd7..320c4739f9 100644
--- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs
+++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs
@@ -57,13 +57,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
 
         public async Task StartAsync(IConnectionHandler connectionHandler)
         {
+            Exception sendError = null;
             try
             {
                 connectionHandler.OnConnection(this);
 
                 // Spawn send and receive logic
                 Task receiveTask = DoReceive();
-                Task sendTask = DoSend();
+                Task<Exception> sendTask = DoSend();
 
                 // If the sending task completes then close the receive
                 // We don't need to do this in the other direction because the kestrel
@@ -76,7 +77,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
 
                 // Now wait for both to complete
                 await receiveTask;
-                await sendTask;
+                sendError = await sendTask;
 
                 // Dispose the socket(should noop if already called)
                 _socket.Dispose();
@@ -85,6 +86,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
             {
                 _trace.LogError(0, ex, $"Unexpected exception in {nameof(SocketConnection)}.{nameof(StartAsync)}.");
             }
+            finally
+            {
+                // Complete the output after disposing the socket
+                Output.Complete(sendError);
+            }
         }
 
         private async Task DoReceive()
@@ -181,7 +187,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
             }
         }
 
-        private async Task DoSend()
+        private async Task<Exception> DoSend()
         {
             Exception error = null;
 
@@ -233,8 +239,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
             }
             finally
             {
-                Output.Complete(error);
-
                 // Make sure to close the connection only after the _aborted flag is set.
                 // Without this, the RequestsCanBeAbortedMidRead test will sometimes fail when
                 // a BadHttpRequestException is thrown instead of a TaskCanceledException.
@@ -242,6 +246,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
                 _trace.ConnectionWriteFin(ConnectionId);
                 _socket.Shutdown(SocketShutdown.Both);
             }
+
+            return error;
         }
     }
 }