From acfcafb6e1ce4a11dd23f09e38b7f46ef3fdc50f Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 29 Aug 2016 17:01:11 -0700 Subject: [PATCH] Ensure MockLibuv.OnPostTask doesn't complete too early --- .../Internal/Infrastructure/KestrelThread.cs | 15 ++++-- .../SocketOutputTests.cs | 50 +++++++++++-------- .../TestHelpers/MockLibuv.cs | 20 +++++--- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs index 4d76a18c67..6d46aba889 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs @@ -19,13 +19,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal /// public class KestrelThread { + private static readonly Action _postCallbackAdapter = (callback, state) => ((Action)callback).Invoke(state); + private static readonly Action _postAsyncCallbackAdapter = (callback, state) => ((Action)callback).Invoke(state); + // maximum times the work queues swapped and are processed in a single pass // as completing a task may immediately have write data to put on the network // otherwise it needs to wait till the next pass of the libuv loop - private const int _maxLoops = 8; - - private static readonly Action _postCallbackAdapter = (callback, state) => ((Action)callback).Invoke(state); - private static readonly Action _postAsyncCallbackAdapter = (callback, state) => ((Action)callback).Invoke(state); + private readonly int _maxLoops = 8; private readonly KestrelEngine _engine; private readonly IApplicationLifetime _appLifetime; @@ -69,6 +69,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal ConnectionManager = new ConnectionManager(this, _threadPool); } + // For testing + internal KestrelThread(KestrelEngine engine, int maxLoops) + : this(engine) + { + _maxLoops = maxLoops; + } + public UvLoopHandle Loop { get { return _loop; } } public MemoryPool Memory { get; } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketOutputTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketOutputTests.cs index e180dc77a3..2f16107a8a 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketOutputTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketOutputTests.cs @@ -26,9 +26,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var mockLibuv = new MockLibuv(); using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -70,9 +71,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -141,9 +143,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -223,9 +226,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -339,9 +343,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -431,9 +436,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -509,9 +515,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -560,9 +567,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -630,9 +638,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); @@ -675,9 +684,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) { - kestrelEngine.Start(count: 1); + var kestrelThread = new KestrelThread(kestrelEngine, maxLoops: 1); + kestrelEngine.Threads.Add(kestrelThread); + await kestrelThread.StartAsync(); - var kestrelThread = kestrelEngine.Threads[0]; var socket = new MockSocket(mockLibuv, kestrelThread.Loop.ThreadId, new TestKestrelTrace()); var trace = new KestrelTrace(new TestKestrelTrace()); var ltp = new SynchronousThreadPool(); diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockLibuv.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockLibuv.cs index a2e34385ad..fcdf349003 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockLibuv.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockLibuv.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers private readonly object _postLock = new object(); private TaskCompletionSource _onPostTcs = new TaskCompletionSource(); private bool _completedOnPostTcs; + private bool _sendCalled; private bool _stopLoop; private readonly ManualResetEventSlim _loopWh = new ManualResetEventSlim(); @@ -47,6 +48,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers PostCount++; + _sendCalled = true; _loopWh.Set(); } @@ -67,23 +69,29 @@ namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers { _loopWh.Wait(); KestrelThreadBlocker.Wait(); - TaskCompletionSource onPostTcs; + TaskCompletionSource onPostTcs = null; lock (_postLock) { + _sendCalled = false; _loopWh.Reset(); _onPost(_postHandle.InternalGetHandle()); - // Ensure any subsequent calls to uv_async_send - // create a new _onPostTcs to be completed. - onPostTcs = _onPostTcs; - _completedOnPostTcs = true; + // Allow the loop to be run again before completing + // _onPostTcs given a nested uv_async_send call. + if (!_sendCalled) + { + // Ensure any subsequent calls to uv_async_send + // create a new _onPostTcs to be completed. + _completedOnPostTcs = true; + onPostTcs = _onPostTcs; + } } // Calling TrySetResult outside the lock to avoid deadlock // when the code attempts to call uv_async_send after awaiting // OnPostTask. - onPostTcs.TrySetResult(null); + onPostTcs?.TrySetResult(null); } return 0;