Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileConfigurationProvider.Dispose should dispose FileProvider when it owns it #86455

Merged
merged 1 commit into from May 19, 2023

Conversation

adamsitnik
Copy link
Member

The app that I've used for leak detection and confirmation of the fix:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="7.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration.Xml" Version="7.0.0" />
  </ItemGroup>
  <ItemGroup>
    <None Update="settings.xml">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </None>
  </ItemGroup>
</Project>
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
	<MySettings>
		<SomeSetting>Test</SomeSetting>
		<Another>true</Another>
	</MySettings>
</configuration>
using Microsoft.Extensions.Configuration;

namespace ConfigLeak
{
    internal class Program
    {
        static void Main()
        {
            const int iterations = 2;

            Console.WriteLine("Press a key");
            Console.ReadKey();

            for (var i = 0; i < iterations; i++)
            {
                Test();

                Console.WriteLine("Take a snapshot and press a key");
                Console.ReadKey();
            }
        }

        static void Test()
        {
            var config = new ConfigurationBuilder().AddXmlFile("settings.xml", false, true).Build();

            (config as IDisposable)?.Dispose();
        }
    }
}

Before:

image

After:

image

The only tricky thing is that the FileProvider can be also provided by the user, so we can not just always dispose it.

fixes #86146

@ghost
Copy link

ghost commented May 18, 2023

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

The app that I've used for leak detection and confirmation of the fix:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="7.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration.Xml" Version="7.0.0" />
  </ItemGroup>
  <ItemGroup>
    <None Update="settings.xml">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </None>
  </ItemGroup>
</Project>
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
	<MySettings>
		<SomeSetting>Test</SomeSetting>
		<Another>true</Another>
	</MySettings>
</configuration>
using Microsoft.Extensions.Configuration;

namespace ConfigLeak
{
    internal class Program
    {
        static void Main()
        {
            const int iterations = 2;

            Console.WriteLine("Press a key");
            Console.ReadKey();

            for (var i = 0; i < iterations; i++)
            {
                Test();

                Console.WriteLine("Take a snapshot and press a key");
                Console.ReadKey();
            }
        }

        static void Test()
        {
            var config = new ConfigurationBuilder().AddXmlFile("settings.xml", false, true).Build();

            (config as IDisposable)?.Dispose();
        }
    }
}

Before:

image

After:

image

The only tricky thing is that the FileProvider can be also provided by the user, so we can not just always dispose it.

fixes #86146

Author: adamsitnik
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: 8.0.0

@adamsitnik adamsitnik changed the title FileConfigurationProvider.Dispose should dispose FileProvider when it ows it FileConfigurationProvider.Dispose should dispose FileProvider when it owns it May 18, 2023
@adamsitnik adamsitnik merged commit 63fad3c into dotnet:main May 19, 2023
102 of 105 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak after configuration disposal from PhysicalFileProvider
2 participants