Skip to content

Conversation

@adamsitnik
Copy link
Member

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

Details
<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:

Details
<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
@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