From 7648e99092e19e56e2650953542ced9903eb377d Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 25 Mar 2021 12:09:20 -0700 Subject: [PATCH] Dispose the ContentFileProvider when the host is disposed (#50181) * Dispose the ContentFileProvider when the host is disposed - Today we don't and it results in a leak when hosts are created and destroyed. * Added test --- .../src/HostBuilder.cs | 5 +- .../src/Internal/Host.cs | 49 +++++++++++++++---- .../tests/UnitTests/HostBuilderTests.cs | 24 +++++++++ 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs b/src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs index ecbaebcf9faa64..1cc812f6f8f199 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs @@ -30,6 +30,7 @@ public class HostBuilder : IHostBuilder private HostBuilderContext _hostBuilderContext; private HostingEnvironment _hostingEnvironment; private IServiceProvider _appServices; + private PhysicalFileProvider _defaultProvider; /// /// A central location for sharing state between components during the host building process. @@ -162,7 +163,7 @@ private void CreateHostingEnvironment() _hostingEnvironment.ApplicationName = Assembly.GetEntryAssembly()?.GetName().Name; } - _hostingEnvironment.ContentRootFileProvider = new PhysicalFileProvider(_hostingEnvironment.ContentRootPath); + _hostingEnvironment.ContentRootFileProvider = _defaultProvider = new PhysicalFileProvider(_hostingEnvironment.ContentRootPath); } private string ResolveContentRootPath(string contentRootPath, string basePath) @@ -219,6 +220,8 @@ private void CreateServiceProvider() services.AddSingleton(_ => { return new Internal.Host(_appServices, + _hostingEnvironment, + _defaultProvider, _appServices.GetRequiredService(), _appServices.GetRequiredService>(), _appServices.GetRequiredService(), diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs index 04b1c1b003525b..2d087b251407bf 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -18,13 +19,23 @@ internal class Host : IHost, IAsyncDisposable private readonly IHostLifetime _hostLifetime; private readonly ApplicationLifetime _applicationLifetime; private readonly HostOptions _options; + private readonly IHostEnvironment _hostEnvironment; + private readonly PhysicalFileProvider _defaultProvider; private IEnumerable _hostedServices; - public Host(IServiceProvider services, IHostApplicationLifetime applicationLifetime, ILogger logger, - IHostLifetime hostLifetime, IOptions options) + public Host(IServiceProvider services, + IHostEnvironment hostEnvironment, + PhysicalFileProvider defaultProvider, + IHostApplicationLifetime applicationLifetime, + ILogger logger, + IHostLifetime hostLifetime, + IOptions options) { Services = services ?? throw new ArgumentNullException(nameof(services)); _applicationLifetime = (applicationLifetime ?? throw new ArgumentNullException(nameof(applicationLifetime))) as ApplicationLifetime; + _hostEnvironment = hostEnvironment; + _defaultProvider = defaultProvider; + if (_applicationLifetime is null) { throw new ArgumentException("Replacing IHostApplicationLifetime is not supported.", nameof(applicationLifetime)); @@ -131,14 +142,34 @@ public async Task StopAsync(CancellationToken cancellationToken = default) public async ValueTask DisposeAsync() { - switch (Services) + // The user didn't change the ContentRootFileProvider instance, we can dispose it + if (ReferenceEquals(_hostEnvironment.ContentRootFileProvider, _defaultProvider)) + { + // Dispose the content provider + await DisposeAsync(_hostEnvironment.ContentRootFileProvider).ConfigureAwait(false); + } + else { - case IAsyncDisposable asyncDisposable: - await asyncDisposable.DisposeAsync().ConfigureAwait(false); - break; - case IDisposable disposable: - disposable.Dispose(); - break; + // In the rare case that the user replaced the ContentRootFileProvider, dispose it and the one + // we originally created + await DisposeAsync(_hostEnvironment.ContentRootFileProvider).ConfigureAwait(false); + await DisposeAsync(_defaultProvider).ConfigureAwait(false); + } + + // Dispose the service provider + await DisposeAsync(Services).ConfigureAwait(false); + + static async ValueTask DisposeAsync(object o) + { + switch (o) + { + case IAsyncDisposable asyncDisposable: + await asyncDisposable.DisposeAsync().ConfigureAwait(false); + break; + case IDisposable disposable: + disposable.Dispose(); + break; + } } } } diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostBuilderTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostBuilderTests.cs index e4b793849acf8d..d1e61255c0618e 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostBuilderTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostBuilderTests.cs @@ -11,6 +11,7 @@ using Microsoft.Extensions.Hosting.Fakes; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; using Xunit; namespace Microsoft.Extensions.Hosting.Tests @@ -531,6 +532,20 @@ public void BuilderPropertiesAreAvailableInBuilderAndContext() using (hostBuilder.Build()) { } } + [Fact] + public void DisposingHostDisposesContentFileProvider() + { + var host = new HostBuilder() + .Build(); + + var env = host.Services.GetRequiredService(); + var fileProvider = new FakeFileProvider(); + env.ContentRootFileProvider = fileProvider; + + host.Dispose(); + Assert.True(fileProvider.Disposed); + } + [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public void HostServicesSameServiceProviderAsInHostBuilder() @@ -544,6 +559,15 @@ public void HostServicesSameServiceProviderAsInHostBuilder() Assert.Same(appServicesFromHostBuilder, host.Services); } + private class FakeFileProvider : IFileProvider, IDisposable + { + public bool Disposed { get; set; } + public void Dispose() { Disposed = true; } + public IDirectoryContents GetDirectoryContents(string subpath) => throw new NotImplementedException(); + public IFileInfo GetFileInfo(string subpath) => throw new NotImplementedException(); + public IChangeToken Watch(string filter) => throw new NotImplementedException(); + } + private class ServiceC { public ServiceC(ServiceD serviceD) { }