Dispose configuration in generic host & host builder

[poke]

This is a follow-up to #861 which covered the configuration part that made configuration providers and the configuration root disposable. What’s left now is the actual code in Host and HostBuilder (as well as WebHost and WebHostBuilder for backwards compatibility) that disposes the configuration.

I have tried to look for a good way to do this and I’m honestly struggling a bit with this. The problem is overall that Host (unlike WebHost) does not have a direct reference to the configuration. The configuration is built by the HostBuilder and added to the service collection there, and then the built service provider is only passed to the Host. The host itself does not access the configuration itself though. This leaves us with a few options:

1. Retrieve the configuration in the Host to be able to dispose it. So either Services.GetService<IConfiguration>() (in the constructor or directly in Dispose before actually disposing the service collection), or by taking a direct dependency on IConfiguration in the constructor. E.g.:

   public void Dispose()
   {
       (Services.GetService<IConfiguration>()> as IDisposable)?.Dispose();
       (Services as IDisposable)?.Dispose();
   }
   

   This feels a bit odd though since Host does not actually use the configuration and from its design it also does not actually care about whether there is a configuration object or not. It would be perfectly fine to create a host without the HostBuilder, and without preparing a configuration object. So it wouldn’t feel perfectly right for the host to check for a configuration root and dispose it, just in case if there is one.

2. Introduce a concept of “objects that should be disposed with the host”. In addition to registering the configuration as an IConfiguration with the service collection, the host builder could also register the same instance with some wrapper that marks it to get disposed with the host. The host could then retrieve all the registered objects and dispose them. This could look like this:

   // in HostBuilder
   services.AddSingleton(_appConfiguration);
   services.AddSingleton(new DisposeWithHost(_appConfiguration));
   
   // in Host.Dispose
   foreach (var disposable in Services.GetServices<DisposeWithHost>())
   {
       (disposable.Instance as IDisposable)?.Dispose();
   }
   

   This would have the added benefit over the first solution that this would not introduce a direct dependency to the configuration inside the host. And the same concept could also be used for other things that should be disposed (e.g. the file providers inside HostingEnvironment).

3. Introduce a way for DI to be able to register a singleton instance while transferring the ownership to the DI container. Right now, singletons will get disposed if they are created by the DI container, i.e. when they aren’t registered with an instance but rather a factory or an implementation type. Because instances aren’t owned by the DI container, it will not dispose them automatically (as per Do not dispose singleton instances aspnet/DependencyInjection#462).

   If there was a way to tell the DI container to register those instances as singletons while taking ownership, then the DI container could simply dispose of them. So the resposibility would be moved away from the host and the host builder:

   services.AddSingletonOwnedByContainer(_appConfiguration);
   

   What already works as a workaround here is to register a factory that statically returns the instance, but that does feel a bit wrong:

   services.AddSingleton(_ => _appConfiguration);
   

   On the other hand, this would be very easy to implement and wouldn’t require additional changes with the host since the service provider is already disposed properly.

---

Regardless of this, there is also the open question on what to do with the builder configuration itself. The host builders’ configuration is used last to initialize the app configuration and after that it is never read from again. A host can also be built just once, so we could dispose the configuration at that point. Since there’s also no way to publicly access the configuration from the builder (unlike the WebHostBuilder), it would be safe to do so at this point.

Also, whatever we end up doing for the generic host and host builder, we also need to transfer over to the web host and web host builder in AspNetCore.

I have working prototypes for the first two options, as well as the workaround of the third option. I just need some feedback on what approach we would like to follow here. Of course, if you have another suggestion, I am more than open to that!

/cc @davidfowl

[davidfowl]

@poke I think the cleanest thing might be to inject the IConfigurationRoot into the HostBuilder and dispose it in Dispose of the Host.

[Sudhakar85]

I am running into same issue. I have GenericHost with Structuremap container in console application. When i dispose the host, the application is not exiting and tried with CTRL+C also didn't work.

public static IHostBuilder BuildDummyHost(CommandLineOptions commandLineParams)
{
var host = new HostBuilder()
.UseServiceProviderFactory(new StructureMapServiceProviderFactory(new MainRegistry()))
.ConfigureContainer((hostBuilder, containerBuilder) =>
{
containerBuilder.For<IHostedService>().Use<AppHost>();
});
return host;
}

[davidfowl]

I’m not sure that has anything to do with this issue. It may be a bug in the structure map adapter

[davidfowl]

Moving to preview4

[poke]

@davidfowl Sorry for the delay on this, I have this on track for next week.

[analogrelay]

@poke is this something you're looking at contributing? We may not have the capacity to do this in 3.0 but we'd definitely be able to help you contribute it! If you're not able to contribute it, that's fine, we'll put it on our queue.

[poke]

@anurse Again, I’m so sorry about this! I haven’t had the chance to take a look at this again for a while (far too long :/) but it’s still on my list. I am confident that I don’t require further help for this and will try to get this done by next weekend. I definitely want to have this done in time for 3.0, so I will hurry up now! – Thanks for your reminder!

[analogrelay]

No worries @poke! I'll leave this in 3.0 for now. I just want to set the right expectations here: We think this is good, but we don't have the resources to do it in 3.0. If you can get something together, awesome! We'll happily help you get it merged! Otherwise it will probably have to wait until a future release :)