From b9901c3bfef479dc41d27980c5fefa4fd5b9b53d Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 15 Jul 2015 16:17:59 -0700 Subject: [PATCH] Surface fatal exceptions that stop the event loop - Request an app Shutdown so KestrelEngine gets disposed - Ensure Listener.Dispose doesn't deadlock before the engine can be disposed - Rely on the existing logic to rethrow in the fatal error when the engine gets disposed --- src/Kestrel/ServerFactory.cs | 6 ++- .../Http/Listener.cs | 40 ++++++++++++------- .../Infrastructure/KestrelThread.cs | 4 ++ .../KestrelEngine.cs | 5 ++- .../EngineTests.cs | 6 +-- .../NetworkingTests.cs | 2 +- .../ShutdownNotImplemented.cs | 25 ++++++++++++ .../TestServer.cs | 2 +- 8 files changed, 66 insertions(+), 24 deletions(-) create mode 100644 test/Microsoft.AspNet.Server.KestrelTests/ShutdownNotImplemented.cs diff --git a/src/Kestrel/ServerFactory.cs b/src/Kestrel/ServerFactory.cs index dbe91f9e06..41fb30cd2e 100644 --- a/src/Kestrel/ServerFactory.cs +++ b/src/Kestrel/ServerFactory.cs @@ -18,10 +18,12 @@ namespace Kestrel public class ServerFactory : IServerFactory { private readonly ILibraryManager _libraryManager; + private readonly IApplicationShutdown _appShutdownService; - public ServerFactory(ILibraryManager libraryManager) + public ServerFactory(ILibraryManager libraryManager, IApplicationShutdown appShutdownService) { _libraryManager = libraryManager; + _appShutdownService = appShutdownService; } public IServerInformation Initialize(IConfiguration configuration) @@ -35,7 +37,7 @@ namespace Kestrel { var disposables = new List(); var information = (ServerInformation)serverInformation; - var engine = new KestrelEngine(_libraryManager); + var engine = new KestrelEngine(_libraryManager, _appShutdownService); engine.Start(1); foreach (var address in information.Addresses) { diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/Listener.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/Listener.cs index b2cf408f75..c48e43b18a 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Http/Listener.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Http/Listener.cs @@ -76,22 +76,32 @@ namespace Microsoft.AspNet.Server.Kestrel.Http public void Dispose() { - var tcs = new TaskCompletionSource(); - Thread.Post( - _ => - { - try + // Ensure the event loop is still running. + // If the event loop isn't running and we try to wait on this Post + // to complete, then KestrelEngine will never be disposed and + // the exception that stopped the event loop will never be surfaced. + if (Thread.FatalError == null) + { + var tcs = new TaskCompletionSource(); + Thread.Post( + _ => { - ListenSocket.Dispose(); - tcs.SetResult(0); - } - catch (Exception ex) - { - tcs.SetException(ex); - } - }, - null); - tcs.Task.Wait(); + try + { + ListenSocket.Dispose(); + tcs.SetResult(0); + } + catch (Exception ex) + { + tcs.SetException(ex); + } + }, + null); + + // REVIEW: Should we add a timeout here to be safe? + tcs.Task.Wait(); + } + ListenSocket = null; } } diff --git a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs index 821a8216da..c971eca102 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs @@ -38,6 +38,7 @@ namespace Microsoft.AspNet.Server.Kestrel } public UvLoopHandle Loop { get { return _loop; } } + public ExceptionDispatchInfo FatalError { get { return _closeError; } } public Action, IntPtr> QueueCloseHandle { get; internal set; } @@ -173,6 +174,9 @@ namespace Microsoft.AspNet.Server.Kestrel catch (Exception ex) { _closeError = ExceptionDispatchInfo.Capture(ex); + // Request shutdown so we can rethrow this exception + // in Stop which should be observable. + _engine.AppShutdown.RequestShutdown(); } } diff --git a/src/Microsoft.AspNet.Server.Kestrel/KestrelEngine.cs b/src/Microsoft.AspNet.Server.Kestrel/KestrelEngine.cs index 21b4df4523..394d3a745c 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/KestrelEngine.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/KestrelEngine.cs @@ -13,9 +13,9 @@ namespace Microsoft.AspNet.Server.Kestrel { public class KestrelEngine : IDisposable { - - public KestrelEngine(ILibraryManager libraryManager) + public KestrelEngine(ILibraryManager libraryManager, IApplicationShutdown appShutdownService) { + AppShutdown = appShutdownService; Threads = new List(); Listeners = new List(); Memory = new MemoryPool(); @@ -63,6 +63,7 @@ namespace Microsoft.AspNet.Server.Kestrel public Libuv Libuv { get; private set; } public IMemoryPool Memory { get; set; } + public IApplicationShutdown AppShutdown { get; private set; } public List Threads { get; private set; } public List Listeners { get; private set; } diff --git a/test/Microsoft.AspNet.Server.KestrelTests/EngineTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/EngineTests.cs index fd9960c819..98be42db2e 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/EngineTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/EngineTests.cs @@ -78,7 +78,7 @@ namespace Microsoft.AspNet.Server.KestrelTests [Fact] public async Task EngineCanStartAndStop() { - var engine = new KestrelEngine(LibraryManager); + var engine = new KestrelEngine(LibraryManager, new ShutdownNotImplemented()); engine.Start(1); engine.Dispose(); } @@ -86,7 +86,7 @@ namespace Microsoft.AspNet.Server.KestrelTests [Fact] public async Task ListenerCanCreateAndDispose() { - var engine = new KestrelEngine(LibraryManager); + var engine = new KestrelEngine(LibraryManager, new ShutdownNotImplemented()); engine.Start(1); var started = engine.CreateServer("http", "localhost", 54321, App); started.Dispose(); @@ -97,7 +97,7 @@ namespace Microsoft.AspNet.Server.KestrelTests [Fact] public async Task ConnectionCanReadAndWrite() { - var engine = new KestrelEngine(LibraryManager); + var engine = new KestrelEngine(LibraryManager, new ShutdownNotImplemented()); engine.Start(1); var started = engine.CreateServer("http", "localhost", 54321, App); diff --git a/test/Microsoft.AspNet.Server.KestrelTests/NetworkingTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/NetworkingTests.cs index 3e549f53a8..2cf9c71d05 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/NetworkingTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/NetworkingTests.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNet.Server.KestrelTests Libuv _uv; public NetworkingTests() { - var engine = new KestrelEngine(LibraryManager); + var engine = new KestrelEngine(LibraryManager, new ShutdownNotImplemented()); _uv = engine.Libuv; } diff --git a/test/Microsoft.AspNet.Server.KestrelTests/ShutdownNotImplemented.cs b/test/Microsoft.AspNet.Server.KestrelTests/ShutdownNotImplemented.cs new file mode 100644 index 0000000000..58697d9a3e --- /dev/null +++ b/test/Microsoft.AspNet.Server.KestrelTests/ShutdownNotImplemented.cs @@ -0,0 +1,25 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Threading; +using Microsoft.Framework.Runtime; + +namespace Microsoft.AspNet.Server.KestrelTests +{ + public class ShutdownNotImplemented : IApplicationShutdown + { + public CancellationToken ShutdownRequested + { + get + { + throw new NotImplementedException(); + } + } + + public void RequestShutdown() + { + throw new NotImplementedException(); + } + } +} diff --git a/test/Microsoft.AspNet.Server.KestrelTests/TestServer.cs b/test/Microsoft.AspNet.Server.KestrelTests/TestServer.cs index e2de4d29b0..dba5cddcde 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/TestServer.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/TestServer.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNet.Server.KestrelTests public void Create(Func app) { - _engine = new KestrelEngine(LibraryManager); + _engine = new KestrelEngine(LibraryManager, new ShutdownNotImplemented()); _engine.Start(1); _server = _engine.CreateServer( "http",