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

Memory leak after configuration disposal from PhysicalFileProvider #86146

Open
MaceWindu opened this issue May 12, 2023 · 5 comments · Fixed by #86455
Open

Memory leak after configuration disposal from PhysicalFileProvider #86146

MaceWindu opened this issue May 12, 2023 · 5 comments · Fixed by #86455

Comments

@MaceWindu
Copy link

Description

When application creates many short-living configuration roots, which use files it results in memory leaks around FileSystemWatcher class (in our case we setup test environment for each test and it leads to memory/performance issues as we have around 100k tests).

I've checked code and see that while ConfigurationRoot.Dispose disposes IConfigurationProvider instances, FileConfigurationProvider.Dispose disposes only _changeTokenRegistration and file (folder actually) watch infrastructure in FileConfigurationSource is never disposed.

Reproduction Steps

internal class Program
	{
		static void Main()
		{
			const int iterations = int.MaxValue;

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

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

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

Expected behavior

No memory leaks

Actual behavior

Memory leaks

We need to support two runtimes currently, so results are for .NET7 and NET48

.NET 7 runtime after ~78k iterations
image
where byte[] instances are
image

In NETFX 4.8 situation is even worse (expected, as it doesn't have all improvements to file watcher related code)
image

Regression?

Nope

Known Workarounds

passing external PhysicalFileProvider instance and dispose it explicitly fixes leak completely:

using var fileProvider = new PhysicalFileProvider(AppContext.BaseDirectory);
using var config = new ConfigurationBuilder().AddXmlFile(fileProvider, "settings.xml", false, true).Build() as IDisposable;

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 12, 2023
@ghost
Copy link

ghost commented May 12, 2023

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

Issue Details

Description

When application creates many short-living configuration roots, which use files it results in memory leaks around FileSystemWatcher class (in our case we setup test environment for each test and it leads to memory/performance issues as we have around 100k tests).

I've checked code and see that while ConfigurationRoot.Dispose disposes IConfigurationProvider instances, FileConfigurationProvider.Dispose disposes only _changeTokenRegistration and file (folder actually) watch infrastructure in FileConfigurationSource is never disposed.

Reproduction Steps

internal class Program
	{
		static void Main()
		{
			const int iterations = int.MaxValue;

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

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

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

Expected behavior

No memory leaks

Actual behavior

Memory leaks

We need to support two runtimes currently, so results are for .NET7 and NET48

.NET 7 runtime after ~78k iterations
image
where byte[] instances are
image

In NETFX 4.8 situation is even worse (expected, as it doesn't have all improvements to file watcher related code)
image

Regression?

Nope

Known Workarounds

passing external PhysicalFileProvider instance and dispose it explicitly fixes leak completely:

using var fileProvider = new PhysicalFileProvider(AppContext.BaseDirectory);
using var config = new ConfigurationBuilder().AddXmlFile(fileProvider, "settings.xml", false, true).Build() as IDisposable;

Configuration

No response

Other information

No response

Author: MaceWindu
Assignees: -
Labels:

untriaged, area-Extensions-Configuration

Milestone: -

@ghost
Copy link

ghost commented May 12, 2023

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

Issue Details

Description

When application creates many short-living configuration roots, which use files it results in memory leaks around FileSystemWatcher class (in our case we setup test environment for each test and it leads to memory/performance issues as we have around 100k tests).

I've checked code and see that while ConfigurationRoot.Dispose disposes IConfigurationProvider instances, FileConfigurationProvider.Dispose disposes only _changeTokenRegistration and file (folder actually) watch infrastructure in FileConfigurationSource is never disposed.

Reproduction Steps

internal class Program
	{
		static void Main()
		{
			const int iterations = int.MaxValue;

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

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

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

Expected behavior

No memory leaks

Actual behavior

Memory leaks

We need to support two runtimes currently, so results are for .NET7 and NET48

.NET 7 runtime after ~78k iterations
image
where byte[] instances are
image

In NETFX 4.8 situation is even worse (expected, as it doesn't have all improvements to file watcher related code)
image

Regression?

Nope

Known Workarounds

passing external PhysicalFileProvider instance and dispose it explicitly fixes leak completely:

using var fileProvider = new PhysicalFileProvider(AppContext.BaseDirectory);
using var config = new ConfigurationBuilder().AddXmlFile(fileProvider, "settings.xml", false, true).Build() as IDisposable;

Configuration

No response

Other information

No response

Author: MaceWindu
Assignees: -
Labels:

untriaged, area-Extensions-FileSystem

Milestone: -

@ghost
Copy link

ghost commented May 12, 2023

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

Issue Details

Description

When application creates many short-living configuration roots, which use files it results in memory leaks around FileSystemWatcher class (in our case we setup test environment for each test and it leads to memory/performance issues as we have around 100k tests).

I've checked code and see that while ConfigurationRoot.Dispose disposes IConfigurationProvider instances, FileConfigurationProvider.Dispose disposes only _changeTokenRegistration and file (folder actually) watch infrastructure in FileConfigurationSource is never disposed.

Reproduction Steps

internal class Program
	{
		static void Main()
		{
			const int iterations = int.MaxValue;

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

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

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

Expected behavior

No memory leaks

Actual behavior

Memory leaks

We need to support two runtimes currently, so results are for .NET7 and NET48

.NET 7 runtime after ~78k iterations
image
where byte[] instances are
image

In NETFX 4.8 situation is even worse (expected, as it doesn't have all improvements to file watcher related code)
image

Regression?

Nope

Known Workarounds

passing external PhysicalFileProvider instance and dispose it explicitly fixes leak completely:

using var fileProvider = new PhysicalFileProvider(AppContext.BaseDirectory);
using var config = new ConfigurationBuilder().AddXmlFile(fileProvider, "settings.xml", false, true).Build() as IDisposable;

Configuration

No response

Other information

No response

Author: MaceWindu
Assignees: -
Labels:

untriaged, area-Extensions-Configuration

Milestone: -

@tarekgh tarekgh added this to the Future milestone May 13, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 13, 2023
@adamsitnik adamsitnik self-assigned this May 18, 2023
adamsitnik added a commit to adamsitnik/runtime that referenced this issue May 18, 2023
@adamsitnik adamsitnik modified the milestones: Future, 8.0.0 May 18, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 18, 2023
@adamsitnik
Copy link
Member

Hi @MaceWindu

Thank you for a bug report with repro case! I was able to repro the bug and send a fix: #86455

I've also created a proposal to help the users avoid getting into the leak: #86456

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 19, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 18, 2023
@adamsitnik
Copy link
Member

I am reopening the issue (I am also going to unlock the conversation) because my fix introduced a bug and I need to revert it.

@adamsitnik adamsitnik reopened this Apr 26, 2024
@dotnet dotnet unlocked this conversation Apr 26, 2024
@adamsitnik adamsitnik modified the milestones: 8.0.0, 9.0.0 Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants