Skip to content

Commit

Permalink
Dispose the ContentFileProvider when the host is disposed (#50181)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
davidfowl authored Mar 25, 2021
1 parent 400311b commit 7648e99
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class HostBuilder : IHostBuilder
private HostBuilderContext _hostBuilderContext;
private HostingEnvironment _hostingEnvironment;
private IServiceProvider _appServices;
private PhysicalFileProvider _defaultProvider;

/// <summary>
/// A central location for sharing state between components during the host building process.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -219,6 +220,8 @@ private void CreateServiceProvider()
services.AddSingleton<IHost>(_ =>
{
return new Internal.Host(_appServices,
_hostingEnvironment,
_defaultProvider,
_appServices.GetRequiredService<IHostApplicationLifetime>(),
_appServices.GetRequiredService<ILogger<Internal.Host>>(),
_appServices.GetRequiredService<IHostLifetime>(),
Expand Down
49 changes: 40 additions & 9 deletions src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<IHostedService> _hostedServices;

public Host(IServiceProvider services, IHostApplicationLifetime applicationLifetime, ILogger<Host> logger,
IHostLifetime hostLifetime, IOptions<HostOptions> options)
public Host(IServiceProvider services,
IHostEnvironment hostEnvironment,
PhysicalFileProvider defaultProvider,
IHostApplicationLifetime applicationLifetime,
ILogger<Host> logger,
IHostLifetime hostLifetime,
IOptions<HostOptions> 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));
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -531,6 +532,20 @@ public void BuilderPropertiesAreAvailableInBuilderAndContext()
using (hostBuilder.Build()) { }
}

[Fact]
public void DisposingHostDisposesContentFileProvider()
{
var host = new HostBuilder()
.Build();

var env = host.Services.GetRequiredService<IHostEnvironment>();
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()
Expand All @@ -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) { }
Expand Down

0 comments on commit 7648e99

Please sign in to comment.