From e9e0cf7325c9814090d69d7e12eab8a14ca26519 Mon Sep 17 00:00:00 2001
From: Stephen Halter <halter73@gmail.com>
Date: Thu, 2 Feb 2017 20:29:33 -0800
Subject: [PATCH] Prevent ODE when ReadStart/Stop() is queued after disposal

---
 .../Internal/Http/BufferSizeControl.cs        | 12 ++----
 .../Internal/Http/Connection.cs               | 42 +++++++++++++++----
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferSizeControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferSizeControl.cs
index 7fbec15ec2..364a1a2cf8 100644
--- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferSizeControl.cs
+++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferSizeControl.cs
@@ -6,18 +6,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
     {
         private readonly long _maxSize;
         private readonly IConnectionControl _connectionControl;
-        private readonly KestrelThread _connectionThread;
 
         private readonly object _lock = new object();
 
         private long _size;
         private bool _connectionPaused;
 
-        public BufferSizeControl(long maxSize, IConnectionControl connectionControl, KestrelThread connectionThread)
+        public BufferSizeControl(long maxSize, IConnectionControl connectionControl)
         {
             _maxSize = maxSize;
             _connectionControl = connectionControl;
-            _connectionThread = connectionThread;
         }
 
         private long Size
@@ -50,9 +48,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                 if (!_connectionPaused && Size >= _maxSize)
                 {
                     _connectionPaused = true;
-                    _connectionThread.Post(
-                        (connectionControl) => ((IConnectionControl)connectionControl).Pause(),
-                        _connectionControl);
+                    _connectionControl.Pause();
                 }
             }
         }
@@ -73,9 +69,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                 if (_connectionPaused && Size < _maxSize)
                 {
                     _connectionPaused = false;
-                    _connectionThread.Post(
-                        (connectionControl) => ((IConnectionControl)connectionControl).Resume(),
-                        _connectionControl);
+                    _connectionControl.Resume();
                 }
             }
         }
diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs
index 9d1c537814..12b978687b 100644
--- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs
+++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs
@@ -57,7 +57,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 
             if (ServerOptions.Limits.MaxRequestBufferSize.HasValue)
             {
-                _bufferSizeControl = new BufferSizeControl(ServerOptions.Limits.MaxRequestBufferSize.Value, this, Thread);
+                _bufferSizeControl = new BufferSizeControl(ServerOptions.Limits.MaxRequestBufferSize.Value, this);
             }
 
             Input = new SocketInput(Thread.Memory, ThreadPool, _bufferSizeControl);
@@ -285,22 +285,46 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
         void IConnectionControl.Pause()
         {
             Log.ConnectionPause(ConnectionId);
-            _socket.ReadStop();
+
+            // Even though this method is called on the event loop already,
+            // post anyway so the ReadStop() call doesn't get reordered
+            // relative to the ReadStart() call made in Resume().
+            Thread.Post(state => ((Connection)state).OnPausePosted(), this);
         }
 
         void IConnectionControl.Resume()
         {
             Log.ConnectionResume(ConnectionId);
-            try
+
+            // This is called from the consuming thread.
+            Thread.Post(state => ((Connection)state).OnResumePosted(), this);
+        }
+
+        private void OnPausePosted()
+        {
+            // It's possible that uv_close was called between the call to Thread.Post() and now.
+            if (!_socket.IsClosed)
             {
-                _socket.ReadStart(_allocCallback, _readCallback, this);
+                _socket.ReadStop();
             }
-            catch (UvException)
+        }
+
+        private void OnResumePosted()
+        {
+            // It's possible that uv_close was called even before the call to Resume().
+            if (!_socket.IsClosed)
             {
-                // ReadStart() can throw a UvException in some cases (e.g. socket is no longer connected).
-                // This should be treated the same as OnRead() seeing a "normalDone" condition.
-                Log.ConnectionReadFin(ConnectionId);
-                Input.IncomingComplete(0, null);
+                try
+                {
+                   _socket.ReadStart(_allocCallback, _readCallback, this);
+                }
+                catch (UvException)
+                {
+                    // ReadStart() can throw a UvException in some cases (e.g. socket is no longer connected).
+                    // This should be treated the same as OnRead() seeing a "normalDone" condition.
+                    Log.ConnectionReadFin(ConnectionId);
+                    Input.IncomingComplete(0, null);
+                }
             }
         }