Handle 0-byte reads correctly (#520).

src/Microsoft.AspNetCore.Server.Kestrel/Http/Connection.cs

@@ -16,9 +16,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
         // Base32 encoding - in ascii sort order for easy text based sorting
         private static readonly string _encode32Chars = "0123456789ABCDEFGHIJKLMNOPQRSTUV";
 
-        private static readonly Action<UvStreamHandle, int, object> _readCallback = 
+        private static readonly Action<UvStreamHandle, int, object> _readCallback =
             (handle, status, state) => ReadCallback(handle, status, state);
-        private static readonly Func<UvStreamHandle, int, object, Libuv.uv_buf_t> _allocCallback = 
+        private static readonly Func<UvStreamHandle, int, object, Libuv.uv_buf_t> _allocCallback =
             (handle, suggestedsize, state) => AllocCallback(handle, suggestedsize, state);
 
         // Seed the _lastConnectionId for this application instance with
@@ -252,8 +252,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
 
         private void OnRead(UvStreamHandle handle, int status)
         {
+            if (status == 0)
+            {
+                // A zero status does not indicate an error or connection end. It indicates
+                // there is no data to be read right now.
+                // See the note at http://docs.libuv.org/en/v1.x/stream.html#c.uv_read_cb.
+                // We need to clean up whatever was allocated by OnAlloc.
+                _rawSocketInput.IncomingDeferred();
+                return;
+            }
+
             var normalRead = status > 0;
-            var normalDone = status == 0 || status == Constants.ECONNRESET || status == Constants.EOF;
+            var normalDone = status == Constants.ECONNRESET || status == Constants.EOF;
             var errorDone = !(normalDone || normalRead);
             var readCount = normalRead ? status : 0;
 

src/Microsoft.AspNetCore.Server.Kestrel/Http/SocketInput.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Diagnostics;
 using System.IO;
 using System.Runtime.CompilerServices;
 using System.Threading;
@@ -117,6 +118,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
             Complete();
         }
 
+        public void IncomingDeferred()
+        {
+            Debug.Assert(_pinned != null);
+
+            if (_pinned != null)
+            {
+                if (_pinned != _tail)
+                {
+                    _memory.Return(_pinned);
+                }
+
+                _pinned = null;
+            }
+        }
+
         private void Complete()
         {
             var awaitableState = Interlocked.Exchange(

test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionTests.cs

@@ -0,0 +1,46 @@
+using System.Threading;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Server.Kestrel;
+using Microsoft.AspNetCore.Server.Kestrel.Http;
+using Microsoft.AspNetCore.Server.Kestrel.Infrastructure;
+using Microsoft.AspNetCore.Server.Kestrel.Networking;
+using Microsoft.AspNetCore.Server.KestrelTests.TestHelpers;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Server.KestrelTests
+{
+    public class ConnectionTests
+    {
+        [Fact]
+        public void DoesNotEndConnectionOnZeroRead()
+        {
+            var mockLibuv = new MockLibuv();
+            
+            using (var memory = new MemoryPool2())
+            using (var engine = new KestrelEngine(mockLibuv, new TestServiceContext()))
+            {
+                engine.Start(count: 1);
+
+                var trace = new TestKestrelTrace();
+                var context = new ListenerContext(new TestServiceContext())
+                {
+                    FrameFactory = connectionContext => new Frame<HttpContext>(
+                        new DummyApplication(httpContext => TaskUtilities.CompletedTask), connectionContext),
+                    Memory2 = memory,
+                    ServerAddress = ServerAddress.FromUrl($"http://localhost:{TestServer.GetNextPort()}"),
+                    Thread = engine.Threads[0]
+                };
+                var socket = new MockSocket(mockLibuv, Thread.CurrentThread.ManagedThreadId, trace);
+                var connection = new Connection(context, socket);
+                connection.Start();
+
+                Libuv.uv_buf_t ignored;
+                mockLibuv.AllocCallback(socket.InternalGetHandle(), 2048, out ignored);
+                mockLibuv.ReadCallback(socket.InternalGetHandle(), 0, ref ignored);
+                Assert.False(connection.SocketInput.RemoteIntakeFin);
+
+                connection.ConnectionControl.End(ProduceEndType.SocketDisconnect);
+            }
+        }
+    }
+}

test/Microsoft.AspNetCore.Server.KestrelTests/SocketOutputTests.cs

@@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 kestrelEngine.Start(count: 1);
 
                 var kestrelThread = kestrelEngine.Threads[0];
-                var socket = new MockSocket(kestrelThread.Loop.ThreadId, new TestKestrelTrace());
+                var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace());
                 var trace = new KestrelTrace(new TestKestrelTrace());
                 var ltp = new LoggingThreadPool(trace);
                 var socketOutput = new SocketOutput(kestrelThread, socket, memory, new MockConnection(), "0", trace, ltp, new Queue<UvWriteReq>());
@@ -90,7 +90,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 kestrelEngine.Start(count: 1);
 
                 var kestrelThread = kestrelEngine.Threads[0];
-                var socket = new MockSocket(kestrelThread.Loop.ThreadId, new TestKestrelTrace());
+                var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace());
                 var trace = new KestrelTrace(new TestKestrelTrace());
                 var ltp = new LoggingThreadPool(trace);
                 var socketOutput = new SocketOutput(kestrelThread, socket, memory, new MockConnection(), "0", trace, ltp, new Queue<UvWriteReq>());
@@ -158,7 +158,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 kestrelEngine.Start(count: 1);
 
                 var kestrelThread = kestrelEngine.Threads[0];
-                var socket = new MockSocket(kestrelThread.Loop.ThreadId, new TestKestrelTrace());
+                var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace());
                 var trace = new KestrelTrace(new TestKestrelTrace());
                 var ltp = new LoggingThreadPool(trace);
                 var socketOutput = new SocketOutput(kestrelThread, socket, memory, new MockConnection(), "0", trace, ltp, new Queue<UvWriteReq>());
@@ -231,7 +231,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 kestrelEngine.Start(count: 1);
 
                 var kestrelThread = kestrelEngine.Threads[0];
-                var socket = new MockSocket(kestrelThread.Loop.ThreadId, new TestKestrelTrace());
+                var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace());
                 var trace = new KestrelTrace(new TestKestrelTrace());
                 var ltp = new LoggingThreadPool(trace);
                 
@@ -341,7 +341,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 kestrelEngine.Start(count: 1);
 
                 var kestrelThread = kestrelEngine.Threads[0];
-                var socket = new MockSocket(kestrelThread.Loop.ThreadId, new TestKestrelTrace());
+                var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace());
                 var trace = new KestrelTrace(new TestKestrelTrace());
                 var ltp = new LoggingThreadPool(trace);
 
@@ -429,7 +429,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 kestrelEngine.Start(count: 1);
 
                 var kestrelThread = kestrelEngine.Threads[0];
-                var socket = new MockSocket(kestrelThread.Loop.ThreadId, new TestKestrelTrace());
+                var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace());
                 var trace = new KestrelTrace(new TestKestrelTrace());
                 var ltp = new LoggingThreadPool(trace);
                 var socketOutput = new SocketOutput(kestrelThread, socket, memory, new MockConnection(), "0", trace, ltp, new Queue<UvWriteReq>());
@@ -515,7 +515,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 kestrelEngine.Start(count: 1);
 
                 var kestrelThread = kestrelEngine.Threads[0];
-                var socket = new MockSocket(kestrelThread.Loop.ThreadId, new TestKestrelTrace());
+                var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace());
                 var trace = new KestrelTrace(new TestKestrelTrace());
                 var ltp = new LoggingThreadPool(trace);
                 var socketOutput = new SocketOutput(kestrelThread, socket, memory, new MockConnection(), "0", trace, ltp, new Queue<UvWriteReq>());
@@ -544,23 +544,5 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                     default(ArraySegment<byte>), default(CancellationToken), socketDisconnect: true);
             }
         }
-
-
-        private class MockSocket : UvStreamHandle
-        {
-            public MockSocket(int threadId, IKestrelTrace logger) : base(logger)
-            {
-                // Set the handle to something other than IntPtr.Zero
-                // so handle.Validate doesn't fail in Libuv.write
-                handle = (IntPtr)1;
-                _threadId = threadId;
-            }
-
-            protected override bool ReleaseHandle()
-            {
-                // No-op
-                return true;
-            }
-        }
     }
 }

test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockLibuv.cs

@@ -17,6 +17,12 @@ namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers
         unsafe public MockLibuv()
             : base(onlyForTesting: true)
         {
+            _onWrite = (socket, buffers, triggerCompleted) =>
+            {
+                triggerCompleted(0);
+                return 0;
+            };
+
             _uv_write = UvWrite;
 
             _uv_async_send = postHandle =>
@@ -69,6 +75,8 @@ namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers
             _uv_walk = (loop, callback, ignore) => 0;
             _uv_err_name = errno => IntPtr.Zero;
             _uv_strerror = errno => IntPtr.Zero;
+            _uv_read_start = UvReadStart;
+            _uv_read_stop = handle => 0;
         }
 
         public Func<UvStreamHandle, int, Action<int>, int> OnWrite
@@ -83,6 +91,17 @@ namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers
             }
         }
 
+        public uv_alloc_cb AllocCallback { get; set; }
+
+        public uv_read_cb ReadCallback { get; set; }
+
+        private int UvReadStart(UvStreamHandle handle, uv_alloc_cb allocCallback, uv_read_cb readCallback)
+        {
+            AllocCallback = allocCallback;
+            ReadCallback = readCallback;
+            return 0;
+        }
+
         unsafe private int UvWrite(UvRequest req, UvStreamHandle handle, uv_buf_t* bufs, int nbufs, uv_write_cb cb)
         {
             return _onWrite(handle, nbufs, status => cb(req.InternalGetHandle(), status));

test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockSocket.cs

@@ -0,0 +1,21 @@
+using System;
+using Microsoft.AspNetCore.Server.Kestrel.Infrastructure;
+using Microsoft.AspNetCore.Server.Kestrel.Networking;
+
+namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers
+{
+    class MockSocket : UvStreamHandle
+    {
+        public MockSocket(Libuv uv, int threadId, IKestrelTrace logger) : base(logger)
+        {
+            CreateMemory(uv, threadId, IntPtr.Size);
+        }
+
+        protected override bool ReleaseHandle()
+        {
+            DestroyMemory(handle);
+            handle = IntPtr.Zero;
+            return true;
+        }
+    }
+}