Wrong disposal of FileConfigurationSource.FileProvider by FileConfigurationProvider.Dispose() on a modification of ConfigurationManager.Sources

[cristalink]

Description

This is a regression in .NET Core 8 from .NET Core 7 caused by commit 63fad3c36fdf36e31ab0dd4c7e32732750390c4a

The lifetimes of FileConfigurationSource and FileConfigurationProvider are different. FileConfigurationSource as an entry in ConfigurationManager.Sources has potentially a longer lifetime than associated FileConfigurationProvider. The latter gets disposed and recreated on virtually every modification of ConfigurationManager.Sources, including Insert(), Remove(), RemoveAt(), etc. Such a modification causes a call to ConfigurationManager.ReloadSources() and, in turn, to ReferenceCountedProviderManager.ReplaceProviders() that disposes FileConfigurationProvider.

However, FileConfigurationProvider.Dispose(), contrary to the good practices of OOP, disposes also FileConfigurationSource.FileProvider. This may result in FileConfigurationSource with permanently disposed FileProvider remaining in ConfigurationManager.Sources. A further modification of ConfigurationManager.Sources may cause access to disposed FileConfigurationSource.FileProvider and thus to ObjectDisposedException.

Reproduction Steps

See the attached repro.

var config = builder.Configuration;
var sources = config.Sources;

// Add a FileConfigurationSource with FileConfigurationSource.OwnsFileProvider set to true (by FileConfigurationSource.ResolveFileProvider()).
var absolutePath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "a.json");
config.AddJsonFile(absolutePath, optional: false, reloadOnChange: true);

var jsonSourceIndex = sources.Count() - 1;
var jsonSource = sources[jsonSourceIndex]; // This is our added configuration source.

// Add two more temporary configuration sources (doesn't matter which), to be removed later on.
config.AddEnvironmentVariables();
config.AddEnvironmentVariables();

if (sources.Count() != (jsonSourceIndex + 2 + 1))
{
    throw new InvalidOperationException($"Sanity check 1");
}

if (!object.ReferenceEquals(jsonSource, sources[jsonSourceIndex]))
{
    throw new InvalidOperationException($"Sanity check 2");
}

// At this stage, jsonSource.FileProvider._disposed is false, which is correct.
if (Debugger.IsAttached)
{
    // Examine jsonSource.FileProvider._disposed here.
    Debugger.Break();
}

// Rearrange 'sources' by removing one of the temporary configuration sources.
// This will cause calls to ConfigurationManager.ReloadSources() and, in turn, ReferenceCountedProviderManager.ReplaceProviders().
sources.RemoveAt(sources.Count - 1);

// Our configuration source is still in the list...
if (!object.ReferenceEquals(jsonSource, sources[jsonSourceIndex]))
{
    throw new InvalidOperationException($"Sanity check 2");
}

// ...but it now contains disposed jsonSource.FileProvider (which is already a problem in itself).
if (Debugger.IsAttached)
{
    // Examine jsonSource.FileProvider._disposed here.
    Debugger.Break();
}

// Rearrange 'sources' again by removing the other temporary configuration source.
// This will cause ObjectDisposedException with the following call stack:
//     System.Private.CoreLib.dll!System.ThrowHelper.ThrowObjectDisposedException(object instance) Line 403	C#
//     System.Private.CoreLib.dll!System.ObjectDisposedException.ThrowIf(bool condition, object instance) Line 61	C#
//   > System.IO.FileSystem.Watcher.dll!System.IO.FileSystemWatcher.EnableRaisingEvents.set(bool value) Line 153	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.TryEnableFileSystemWatcher() Line 437	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.CreateFileChangeToken(string filter) Line 152	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.PhysicalFileProvider.Watch(string filter) Line 362	C#
//     Microsoft.Extensions.Primitives.dll!Microsoft.Extensions.Primitives.ChangeToken.ChangeTokenRegistration<System.Action>.ChangeTokenRegistration(System.Func<Microsoft.Extensions.Primitives.IChangeToken> changeTokenProducer, System.Action<System.Action> changeTokenConsumer, System.Action state) Line 72	C#
//     Microsoft.Extensions.Primitives.dll!Microsoft.Extensions.Primitives.ChangeToken.OnChange(System.Func<Microsoft.Extensions.Primitives.IChangeToken> changeTokenProducer, System.Action changeTokenConsumer) Line 31	C#
//     Microsoft.Extensions.Configuration.FileExtensions.dll!Microsoft.Extensions.Configuration.FileConfigurationProvider.FileConfigurationProvider(Microsoft.Extensions.Configuration.FileConfigurationSource source) Line 34	C#
//     Microsoft.Extensions.Configuration.Json.dll!Microsoft.Extensions.Configuration.Json.JsonConfigurationSource.Build(Microsoft.Extensions.Configuration.IConfigurationBuilder builder) Line 21	C#
//     Microsoft.Extensions.Configuration.dll!Microsoft.Extensions.Configuration.ConfigurationManager.ReloadSources() Line 146	C#
//     WebApplication1.dll!Program.<Main>$(string[] args = {string[0x00000000]}) Line 61	C#
sources.RemoveAt(sources.Count - 1);

WebApplication1.zip

Expected behavior

Modification of ConfigurationManager.Sources should not adversely affect the configuration sources remaining on the list.

Actual behavior

Modification of ConfigurationManager.Sources wrongly causes the disposal of FileConfigurationSource.FileProvider.

Regression?

Yes, it's a regression from .NET Core 7, per the description above.

Known Workarounds

There is no workaround.

Configuration

A simple "ASP.NET Core Web App (Razor Pages)" from a standard Visual Studio template with .NET 8.

Other information

No response

[ghost]

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

Issue Details

---

Description

This is a regression in .NET Core 8 from .NET Core 7 caused by commit 63fad3c36fdf36e31ab0dd4c7e32732750390c4a

The lifetimes of FileConfigurationSource and FileConfigurationProvider are different. FileConfigurationSource as an entry in ConfigurationManager.Sources has potentially a longer lifetime than associated FileConfigurationProvider. The latter gets disposed and recreated on virtually every modification of ConfigurationManager.Sources, including Insert(), Remove(), RemoveAt(), etc. Such a modification causes a call to ConfigurationManager.ReloadSources() and, in turn, to ReferenceCountedProviderManager.ReplaceProviders() that disposes FileConfigurationProvider.

However, FileConfigurationProvider.Dispose(), contrary to the good practices of OOP, disposes also FileConfigurationSource.FileProvider. This may result in FileConfigurationSource with permanently disposed FileProvider remaining in ConfigurationManager.Sources. A further modification of ConfigurationManager.Sources may cause access to disposed FileConfigurationSource.FileProvider and thus to ObjectDisposedException.

Reproduction Steps

See the attached repro.

var config = builder.Configuration;
var sources = config.Sources;

// Add a FileConfigurationSource with FileConfigurationSource.OwnsFileProvider set to true (by FileConfigurationSource.ResolveFileProvider()).
var absolutePath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "a.json");
config.AddJsonFile(absolutePath, optional: false, reloadOnChange: true);

var jsonSourceIndex = sources.Count() - 1;
var jsonSource = sources[jsonSourceIndex]; // This is our added configuration source.

// Add two more temporary configuration sources (doesn't matter which), to be removed later on.
config.AddEnvironmentVariables();
config.AddEnvironmentVariables();

if (sources.Count() != (jsonSourceIndex + 2 + 1))
{
    throw new InvalidOperationException($"Sanity check 1");
}

if (!object.ReferenceEquals(jsonSource, sources[jsonSourceIndex]))
{
    throw new InvalidOperationException($"Sanity check 2");
}

// At this stage, jsonSource.FileProvider._disposed is false, which is correct.
if (Debugger.IsAttached)
{
    // Examine jsonSource.FileProvider._disposed here.
    Debugger.Break();
}

// Rearrange 'sources' by removing one of the temporary configuration sources.
// This will cause calls to ConfigurationManager.ReloadSources() and, in turn, ReferenceCountedProviderManager.ReplaceProviders().
sources.RemoveAt(sources.Count - 1);

// Our configuration source is still in the list...
if (!object.ReferenceEquals(jsonSource, sources[jsonSourceIndex]))
{
    throw new InvalidOperationException($"Sanity check 2");
}

// ...but it now contains disposed jsonSource.FileProvider (which is already a problem in itself).
if (Debugger.IsAttached)
{
    // Examine jsonSource.FileProvider._disposed here.
    Debugger.Break();
}

// Rearrange 'sources' again by removing the other temporary configuration source.
// This will cause ObjectDisposedException with the following call stack:
//     System.Private.CoreLib.dll!System.ThrowHelper.ThrowObjectDisposedException(object instance) Line 403	C#
//     System.Private.CoreLib.dll!System.ObjectDisposedException.ThrowIf(bool condition, object instance) Line 61	C#
//   > System.IO.FileSystem.Watcher.dll!System.IO.FileSystemWatcher.EnableRaisingEvents.set(bool value) Line 153	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.TryEnableFileSystemWatcher() Line 437	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.CreateFileChangeToken(string filter) Line 152	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.PhysicalFileProvider.Watch(string filter) Line 362	C#
//     Microsoft.Extensions.Primitives.dll!Microsoft.Extensions.Primitives.ChangeToken.ChangeTokenRegistration<System.Action>.ChangeTokenRegistration(System.Func<Microsoft.Extensions.Primitives.IChangeToken> changeTokenProducer, System.Action<System.Action> changeTokenConsumer, System.Action state) Line 72	C#
//     Microsoft.Extensions.Primitives.dll!Microsoft.Extensions.Primitives.ChangeToken.OnChange(System.Func<Microsoft.Extensions.Primitives.IChangeToken> changeTokenProducer, System.Action changeTokenConsumer) Line 31	C#
//     Microsoft.Extensions.Configuration.FileExtensions.dll!Microsoft.Extensions.Configuration.FileConfigurationProvider.FileConfigurationProvider(Microsoft.Extensions.Configuration.FileConfigurationSource source) Line 34	C#
//     Microsoft.Extensions.Configuration.Json.dll!Microsoft.Extensions.Configuration.Json.JsonConfigurationSource.Build(Microsoft.Extensions.Configuration.IConfigurationBuilder builder) Line 21	C#
//     Microsoft.Extensions.Configuration.dll!Microsoft.Extensions.Configuration.ConfigurationManager.ReloadSources() Line 146	C#
//     WebApplication1.dll!Program.<Main>$(string[] args = {string[0x00000000]}) Line 61	C#
sources.RemoveAt(sources.Count - 1);

WebApplication1.zip

Expected behavior

Modification of ConfigurationManager.Sources should not adversely affect the configuration sources remaining on the list.

Actual behavior

Modification of ConfigurationManager.Sources wrongly causes the disposal of FileConfigurationSource.FileProvider.

Regression?

Yes, it's a regression from .NET Core 7, per the description above.

Known Workarounds

There is no workaround.

Configuration

A simple "ASP.NET Core Web App (Razor Pages)" from a standard Visual Studio template with .NET 8.

Other information

No response

Author:	cristalink
Assignees:	-
Labels:	area-Extensions-Configuration
Milestone:	-

[ghost]

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

Issue Details

---

Description

This is a regression in .NET Core 8 from .NET Core 7 caused by commit 63fad3c36fdf36e31ab0dd4c7e32732750390c4a

The lifetimes of FileConfigurationSource and FileConfigurationProvider are different. FileConfigurationSource as an entry in ConfigurationManager.Sources has potentially a longer lifetime than associated FileConfigurationProvider. The latter gets disposed and recreated on virtually every modification of ConfigurationManager.Sources, including Insert(), Remove(), RemoveAt(), etc. Such a modification causes a call to ConfigurationManager.ReloadSources() and, in turn, to ReferenceCountedProviderManager.ReplaceProviders() that disposes FileConfigurationProvider.

However, FileConfigurationProvider.Dispose(), contrary to the good practices of OOP, disposes also FileConfigurationSource.FileProvider. This may result in FileConfigurationSource with permanently disposed FileProvider remaining in ConfigurationManager.Sources. A further modification of ConfigurationManager.Sources may cause access to disposed FileConfigurationSource.FileProvider and thus to ObjectDisposedException.

Reproduction Steps

See the attached repro.

var config = builder.Configuration;
var sources = config.Sources;

// Add a FileConfigurationSource with FileConfigurationSource.OwnsFileProvider set to true (by FileConfigurationSource.ResolveFileProvider()).
var absolutePath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "a.json");
config.AddJsonFile(absolutePath, optional: false, reloadOnChange: true);

var jsonSourceIndex = sources.Count() - 1;
var jsonSource = sources[jsonSourceIndex]; // This is our added configuration source.

// Add two more temporary configuration sources (doesn't matter which), to be removed later on.
config.AddEnvironmentVariables();
config.AddEnvironmentVariables();

if (sources.Count() != (jsonSourceIndex + 2 + 1))
{
    throw new InvalidOperationException($"Sanity check 1");
}

if (!object.ReferenceEquals(jsonSource, sources[jsonSourceIndex]))
{
    throw new InvalidOperationException($"Sanity check 2");
}

// At this stage, jsonSource.FileProvider._disposed is false, which is correct.
if (Debugger.IsAttached)
{
    // Examine jsonSource.FileProvider._disposed here.
    Debugger.Break();
}

// Rearrange 'sources' by removing one of the temporary configuration sources.
// This will cause calls to ConfigurationManager.ReloadSources() and, in turn, ReferenceCountedProviderManager.ReplaceProviders().
sources.RemoveAt(sources.Count - 1);

// Our configuration source is still in the list...
if (!object.ReferenceEquals(jsonSource, sources[jsonSourceIndex]))
{
    throw new InvalidOperationException($"Sanity check 2");
}

// ...but it now contains disposed jsonSource.FileProvider (which is already a problem in itself).
if (Debugger.IsAttached)
{
    // Examine jsonSource.FileProvider._disposed here.
    Debugger.Break();
}

// Rearrange 'sources' again by removing the other temporary configuration source.
// This will cause ObjectDisposedException with the following call stack:
//     System.Private.CoreLib.dll!System.ThrowHelper.ThrowObjectDisposedException(object instance) Line 403	C#
//     System.Private.CoreLib.dll!System.ObjectDisposedException.ThrowIf(bool condition, object instance) Line 61	C#
//   > System.IO.FileSystem.Watcher.dll!System.IO.FileSystemWatcher.EnableRaisingEvents.set(bool value) Line 153	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.TryEnableFileSystemWatcher() Line 437	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.CreateFileChangeToken(string filter) Line 152	C#
//     Microsoft.Extensions.FileProviders.Physical.dll!Microsoft.Extensions.FileProviders.PhysicalFileProvider.Watch(string filter) Line 362	C#
//     Microsoft.Extensions.Primitives.dll!Microsoft.Extensions.Primitives.ChangeToken.ChangeTokenRegistration<System.Action>.ChangeTokenRegistration(System.Func<Microsoft.Extensions.Primitives.IChangeToken> changeTokenProducer, System.Action<System.Action> changeTokenConsumer, System.Action state) Line 72	C#
//     Microsoft.Extensions.Primitives.dll!Microsoft.Extensions.Primitives.ChangeToken.OnChange(System.Func<Microsoft.Extensions.Primitives.IChangeToken> changeTokenProducer, System.Action changeTokenConsumer) Line 31	C#
//     Microsoft.Extensions.Configuration.FileExtensions.dll!Microsoft.Extensions.Configuration.FileConfigurationProvider.FileConfigurationProvider(Microsoft.Extensions.Configuration.FileConfigurationSource source) Line 34	C#
//     Microsoft.Extensions.Configuration.Json.dll!Microsoft.Extensions.Configuration.Json.JsonConfigurationSource.Build(Microsoft.Extensions.Configuration.IConfigurationBuilder builder) Line 21	C#
//     Microsoft.Extensions.Configuration.dll!Microsoft.Extensions.Configuration.ConfigurationManager.ReloadSources() Line 146	C#
//     WebApplication1.dll!Program.<Main>$(string[] args = {string[0x00000000]}) Line 61	C#
sources.RemoveAt(sources.Count - 1);

WebApplication1.zip

Expected behavior

Modification of ConfigurationManager.Sources should not adversely affect the configuration sources remaining on the list.

Actual behavior

Modification of ConfigurationManager.Sources wrongly causes the disposal of FileConfigurationSource.FileProvider.

Regression?

Yes, it's a regression from .NET Core 7, per the description above.

Known Workarounds

There is no workaround.

Configuration

A simple "ASP.NET Core Web App (Razor Pages)" from a standard Visual Studio template with .NET 8.

Other information

No response

Author:	cristalink
Assignees:	-
Labels:	untriaged, area-Extensions-FileSystem
Milestone:	-

[adamsitnik]

@cristalink big thanks for a very detailed bug report and apologies for missing the issue. Before I come up with a fix I can provide a workaround: create IFileProvider on your own and use AddJsonFile overload that takes an instance of it rather than the string path.

Example:

PhysicalFileProvider fileProvider = new(Path.GetDirectoryName(filePath));
JsonConfigurationProvider configurationProvider = new(new JsonConfigurationSource()
{
    Path = filePath,
    FileProvider = fileProvider
});

IConfigurationRoot config = new ConfigurationBuilder()
    .AddJsonFile(configurationProvider.Source.FileProvider, filePath, optional: true, reloadOnChange: false)
    .Build();

Explanation: user provided providers are not disposed by FileConfigurationProvider:

runtime/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs

Lines 166 to 168 in ffb2578

	if (Source.OwnsFileProvider)
	{
	(Source.FileProvider as IDisposable)?.Dispose();