Graceful Server Shutdown

[Tratcher]

Today IServer is in the services container and a graceful shutdown is triggered by disposing the entire container. However, the container does not guarantee which order services get disposed in so services the server (or requests in flight) depend on may get disposed before the server stops, causing strange errors and making shutdown unstable.

Example scenario; the console logger got disposed before the server, causing exceptions when logging on shutdown:

Unhandled Exception: System.AggregateException: An error occurred while writing to logger(s). ---> System.InvalidOperationException: The collection has been marked as complete with regards to additions.
   at System.Collections.Concurrent.BlockingCollection`1.TryAddWithNoTimeValidation(T item, Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at Microsoft.Extensions.Logging.Console.Internal.ConsoleLoggerProcessor.EnqueueMessage(LogMessageEntry message)
   at Microsoft.Extensions.Logging.Console.ConsoleLogger.WriteMessage(LogLevel logLevel, String logName, Int32 eventId, String message, Exception exception)
   at Microsoft.Extensions.Logging.Console.ConsoleLogger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func`3 formatter)
   at Microsoft.Extensions.Logging.Logger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func`3 formatter)
   --- End of inner exception stack trace ---
   at Microsoft.Extensions.Logging.Logger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func`3 formatter)
   at Microsoft.Extensions.Logging.LoggerExtensions.LogError(ILogger logger, EventId eventId, Exception exception, String message, Object[] args)
   at Microsoft.AspNetCore.Server.HttpSys.LogHelper.LogException(ILogger logger, String location, Exception exception)
   at Microsoft.AspNetCore.Server.HttpSys.HttpSysListener.Dispose(Boolean disposing)
   at Microsoft.AspNetCore.Server.HttpSys.HttpSysListener.Dispose()
   at Microsoft.AspNetCore.Server.HttpSys.MessagePump.Dispose()
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.Dispose()
   at Microsoft.AspNetCore.Hosting.Internal.WebHost.Dispose()
   at Microsoft.AspNetCore.Hosting.WebHostExtensions.Run(IWebHost host, CancellationToken token, String shutdownMessage)
   at Microsoft.AspNetCore.Hosting.WebHostExtensions.Run(IWebHost host)
   at SelfHostServer.Startup.Main(String[] args)

@BrennanConroy is going to get rid of this specific exception by making logging no-op after dispose.

A similar issue caused a deadlock during graceful shutdown between WebListener and an in flight request resolving a service, as the service container is locked during dispose. See aspnet/HttpSysServer#298

[Tratcher]

Proposal: On graceful shutdown dispose the IServer instance explicitly before disposing the service container.

This issue applies to both Kestrel and HttpSys, but the default container disposal ordering may be different.

[halter73]

However, the container does not guarantee which order services get disposed

This is slated to change in 2.0.0: aspnet/DependencyInjection#463

[Tratcher]

I don't think that's adequate to address the issue of letting requests complete gracefully. The server is created before many request services, and disposing those request services while requests are still in flight would still be a peroblem.

Or we stop conflating Stop and Dispose and call Stop for graceful shutdown before Dispose.

[davidfowl]

Or we stop conflating Stop and Dispose and call Stop for graceful shutdown before Dispose.

That sounds more reasonable. Maybe we need a Stop on IServer. It would be a breaking change but it would be a good one.

[halter73]

If we're going to change the IServer interface, we could change the Start method to return an IDisposable instead of implementing IDisposable directly.

[Tratcher]

The server may have resources to dispose even if not started.

Stop could be used for graceful shutdown and Dispose could be used for abortive shutdown.

[halter73]

The problem is that then IServer is not restartable, since there is no IServerFactory.

[Tratcher]

It never has been restartable. Are you saying that if I call Stop on it I should also be able to call Start again?

[halter73]

True, it's never been restartable, but it would be nice if it was.

I'm promoting Start returning an IDisposable. So not only could you stop and restart again, you could Start multiple times without stopping allowing multiple "servers" to be running simultaneously with the same services, but maybe (not necessarily) different applications.

We would have to rethink how endpoints are configured in this world, since using the Features property on IServer wouldn't make much sense (I'm thinking we'd have to add a parameter to Start), but we want to rethink endpoint configuration anyway.

[Tratcher]

Given the WebHostBuilder and WebHost are single use, why make the server restartable?

[muratg]

We'll add a Stop method.

[davidfowl]

This should also solve the graceful shutdown dispose problem described here aspnet/KestrelHttpServer#1501 (that also hit WebListener in 1.0)

[Tratcher]

Feature Creep:

• Make Server Start and Stop fully async with cancellation tokens.
• Add WebHost StartAsync and StopAsync with cancellation tokens (and RunAsync?).
• WebHost.Dispose may call StopAsync if it was not already called

[Tratcher]

Hmm, @davidfowl if WebHost becomes async then you lose your excuses about not making Startup async.

[Tratcher]

Should this async extend to IHostedService.Start/Stop?