diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs index 4d12a2174f..cdcaccfc36 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs @@ -47,14 +47,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal if (!buffer.IsEmpty) { var writeReq = _writeReqPool.Allocate(); - var writeResult = await writeReq.WriteAsync(_socket, buffer); - _writeReqPool.Return(writeReq); - LogWriteInfo(writeResult.Status, writeResult.Error); - - if (writeResult.Error != null) + try { - throw writeResult.Error; + var writeResult = await writeReq.WriteAsync(_socket, buffer); + + LogWriteInfo(writeResult.Status, writeResult.Error); + + if (writeResult.Error != null) + { + throw writeResult.Error; + } + } + finally + { + // Make sure we return the writeReq to the pool + _writeReqPool.Return(writeReq); } } @@ -104,15 +112,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal _log.ConnectionWriteFin(_connectionId); var shutdownReq = new UvShutdownReq(_log); - shutdownReq.Init(_thread.Loop); - shutdownReq.Shutdown(_socket, (req, status, state) => + try { - req.Dispose(); - _log.ConnectionWroteFin(_connectionId, status); + shutdownReq.Init(_thread); + shutdownReq.Shutdown(_socket, (req, status, state) => + { + req.Dispose(); + _log.ConnectionWroteFin(_connectionId, status); - tcs.TrySetResult(null); - }, - this); + tcs.TrySetResult(null); + }, + this); + } + catch (Exception) + { + shutdownReq.Dispose(); + throw; + } return tcs.Task; } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs index d183f20f72..d4db9803a0 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs @@ -3,13 +3,14 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networking; using Microsoft.AspNetCore.Server.Kestrel.Internal.System.IO.Pipelines; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networking; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal @@ -76,6 +77,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public WriteReqPool WriteReqPool { get; } +#if DEBUG + public List Requests { get; } = new List(); +#endif + public ExceptionDispatchInfo FatalError { get { return _closeError; } } public Action, IntPtr> QueueCloseHandle { get; } @@ -141,6 +146,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal } } +#if DEBUG + private void CheckUvReqLeaks() + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + // Detect leaks in UvRequest objects + foreach (var request in Requests) + { + Debug.Assert(request.Target == null, $"{request.Target?.GetType()} object is still alive."); + } + } +#endif + private async Task DisposeConnectionsAsync() { // Close and wait for all connections @@ -299,6 +319,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal WriteReqPool.Dispose(); thisHandle.Free(); _threadTcs.SetResult(null); + +#if DEBUG + // Check for handle leaks after disposing everything + CheckUvReqLeaks(); +#endif } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerPrimary.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerPrimary.cs index d1cdcd150f..45145aadc5 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerPrimary.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerPrimary.cs @@ -105,17 +105,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal DetachFromIOCP(socket); var dispatchPipe = _dispatchPipes[index]; var write = new UvWriteReq(Log); - write.Init(Thread.Loop); - write.Write2( - dispatchPipe, - _dummyMessage, - socket, - (write2, status, error, state) => - { - write2.Dispose(); - ((UvStreamHandle)state).Dispose(); - }, - socket); + try + { + write.Init(Thread); + write.Write2( + dispatchPipe, + _dummyMessage, + socket, + (write2, status, error, state) => + { + write2.Dispose(); + ((UvStreamHandle)state).Dispose(); + }, + socket); + } + catch (UvException) + { + write.Dispose(); + throw; + } } } @@ -230,7 +238,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal if (correctMessage) { - _listener._dispatchPipes.Add((UvPipeHandle) dispatchPipe); + _listener._dispatchPipes.Add((UvPipeHandle)dispatchPipe); dispatchPipe.ReadStop(); _bufHandle.Free(); } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs index ced08d8126..0d45492472 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs @@ -59,11 +59,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal private void StartedCallback(TaskCompletionSource tcs) { + var connect = new UvConnectRequest(Log); try { DispatchPipe.Init(Thread.Loop, Thread.QueueCloseHandle, true); - var connect = new UvConnectRequest(Log); - connect.Init(Thread.Loop); + connect.Init(Thread); connect.Connect( DispatchPipe, _pipeName, @@ -73,6 +73,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal catch (Exception ex) { DispatchPipe.Dispose(); + connect.Dispose(); tcs.SetException(ex); } } @@ -101,7 +102,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal (handle, status2, state) => ((ListenerSecondary)state).ReadStartCallback(handle, status2), this); - writeReq.Init(Thread.Loop); + writeReq.Init(Thread); var result = await writeReq.WriteAsync( DispatchPipe, new ArraySegment>(new [] { new ArraySegment(_pipeMessage) })); @@ -117,10 +118,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal } catch (Exception ex) { - writeReq.Dispose(); DispatchPipe.Dispose(); tcs.SetException(ex); } + finally + { + writeReq.Dispose(); + } } private void ReadStartCallback(UvStreamHandle handle, int status) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvConnectRequest.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvConnectRequest.cs index 3e29e42520..0c0371e2bb 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvConnectRequest.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvConnectRequest.cs @@ -20,7 +20,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin { } - public void Init(UvLoopHandle loop) + public override void Init(LibuvThread thread) + { + DangerousInit(thread.Loop); + + base.Init(thread); + } + + public void DangerousInit(UvLoopHandle loop) { var requestSize = loop.Libuv.req_size(LibuvFunctions.RequestType.CONNECT); CreateMemory( @@ -38,14 +45,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin _callback = callback; _state = state; - Pin(); Libuv.pipe_connect(this, pipe, name, _uv_connect_cb); } private static void UvConnectCb(IntPtr ptr, int status) { var req = FromIntPtr(ptr); - req.Unpin(); var callback = req._callback; req._callback = null; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvMemory.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvMemory.cs index dcd5fdec2a..1285777910 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvMemory.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvMemory.cs @@ -17,10 +17,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin protected LibuvFunctions _uv; protected int _threadId; protected readonly ILibuvTrace _log; + private readonly GCHandleType _handleType; - protected UvMemory(ILibuvTrace logger) : base(IntPtr.Zero, true) + protected UvMemory(ILibuvTrace logger, GCHandleType handleType = GCHandleType.Weak) : base(IntPtr.Zero, true) { _log = logger; + _handleType = handleType; } public LibuvFunctions Libuv { get { return _uv; } } @@ -51,7 +53,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin ThreadId = threadId; handle = Marshal.AllocCoTaskMem(size); - *(IntPtr*)handle = GCHandle.ToIntPtr(GCHandle.Alloc(this, GCHandleType.Weak)); + *(IntPtr*)handle = GCHandle.ToIntPtr(GCHandle.Alloc(this, _handleType)); } unsafe protected static void DestroyMemory(IntPtr memory) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvRequest.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvRequest.cs index 3826ea9cde..e11b1aaadf 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvRequest.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvRequest.cs @@ -8,28 +8,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin { public class UvRequest : UvMemory { - private GCHandle _pin; - - protected UvRequest(ILibuvTrace logger) : base (logger) + protected UvRequest(ILibuvTrace logger) : base(logger, GCHandleType.Normal) { } + public virtual void Init(LibuvThread thread) + { +#if DEBUG + // Store weak handles to all UvRequest objects so we can do leak detection + // while running tests + thread.Requests.Add(new WeakReference(this)); +#endif + } + protected override bool ReleaseHandle() { DestroyMemory(handle); handle = IntPtr.Zero; return true; } - - public virtual void Pin() - { - _pin = GCHandle.Alloc(this, GCHandleType.Normal); - } - - public virtual void Unpin() - { - _pin.Free(); - } } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvShutdownReq.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvShutdownReq.cs index 726d1b9867..19af091f01 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvShutdownReq.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvShutdownReq.cs @@ -19,26 +19,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin { } - public void Init(UvLoopHandle loop) + public override void Init(LibuvThread thread) { + var loop = thread.Loop; + CreateMemory( loop.Libuv, loop.ThreadId, loop.Libuv.req_size(LibuvFunctions.RequestType.SHUTDOWN)); + + base.Init(thread); } public void Shutdown(UvStreamHandle handle, Action callback, object state) { _callback = callback; _state = state; - Pin(); _uv.shutdown(this, handle, _uv_shutdown_cb); } private static void UvShutdownCb(IntPtr ptr, int status) { var req = FromIntPtr(ptr); - req.Unpin(); req._callback(req, status, req._state); req._callback = null; req._state = null; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvWriteReq.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvWriteReq.cs index 77b651b2fd..ab14ffdf2b 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvWriteReq.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/Networking/UvWriteReq.cs @@ -31,7 +31,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin { } - public void Init(UvLoopHandle loop) + public override void Init(LibuvThread thread) + { + DangerousInit(thread.Loop); + + base.Init(thread); + } + + public void DangerousInit(UvLoopHandle loop) { var requestSize = loop.Libuv.req_size(LibuvFunctions.RequestType.WRITE); var bufferSize = Marshal.SizeOf() * BUFFER_COUNT; @@ -62,9 +69,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin { try { - // add GCHandle to keeps this SafeHandle alive while request processing - _pins.Add(GCHandle.Alloc(this, GCHandleType.Normal)); - var nBuffers = 0; if (buffer.IsSingleSpan) { @@ -157,9 +161,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin { try { - // add GCHandle to keeps this SafeHandle alive while request processing - _pins.Add(GCHandle.Alloc(this, GCHandleType.Normal)); - var pBuffers = (LibuvFunctions.uv_buf_t*)_bufs; var nBuffers = bufs.Count; if (nBuffers > BUFFER_COUNT) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/WriteReqPool.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/WriteReqPool.cs index 9a7ad2532a..cdc612ee4f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/WriteReqPool.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/WriteReqPool.cs @@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal else { req = new UvWriteReq(_log); - req.Init(_thread.Loop); + req.Init(_thread); } return req; diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs index 07658dcc23..0c9d5b52be 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs @@ -131,7 +131,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests var connectReq = new UvConnectRequest(connectionTrace); pipe.Init(libuvThreadPrimary.Loop, libuvThreadPrimary.QueueCloseHandle); - connectReq.Init(libuvThreadPrimary.Loop); + connectReq.Init(libuvThreadPrimary); connectReq.Connect( pipe, diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/MultipleLoopTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/MultipleLoopTests.cs index 59f7a8f719..e57b16b30a 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/MultipleLoopTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/MultipleLoopTests.cs @@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests } var writeRequest = new UvWriteReq(_logger); - writeRequest.Init(loop); + writeRequest.DangerousInit(loop); await writeRequest.WriteAsync( serverConnectionPipe, @@ -84,7 +84,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests loop2.Init(_uv); clientConnectionPipe.Init(loop2, (a, b) => { }, true); - connect.Init(loop2); + connect.DangerousInit(loop2); connect.Connect(clientConnectionPipe, pipeName, (handle, status, error, state) => { var buf = loop2.Libuv.buf_init(Marshal.AllocHGlobal(8192), 8192); @@ -157,7 +157,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests serverConnectionPipeAcceptedEvent.WaitOne(); var writeRequest = new UvWriteReq(_logger); - writeRequest.Init(loop); + writeRequest.DangerousInit(loop); writeRequest.Write2( serverConnectionPipe, new ArraySegment>(new ArraySegment[] { new ArraySegment(new byte[] { 1, 2, 3, 4 }) }), @@ -182,7 +182,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests loop2.Init(_uv); clientConnectionPipe.Init(loop2, (a, b) => { }, true); - connect.Init(loop2); + connect.DangerousInit(loop2); connect.Connect(clientConnectionPipe, pipeName, (handle, status, error, state) => { connect.Dispose(); diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/NetworkingTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/NetworkingTests.cs index fd103c299f..72aab202ba 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/NetworkingTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests/NetworkingTests.cs @@ -160,7 +160,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests for (var x = 0; x < 2; x++) { var req = new UvWriteReq(_logger); - req.Init(loop); + req.DangerousInit(loop); var block = ReadableBuffer.Create(new byte[] { 65, 66, 67, 68, 69 }); await req.WriteAsync(