Skip to content

Commit 63fad3c

Browse files
adamsitnikadamsitnik
authored
FileConfigurationProvider.Dispose should dispose FileProvider when it ows it (#86455)
fixes #86146
1 parent 8967844 commit 63fad3c

File tree

5 files changed

+142
-6
lines changed

5 files changed

+142
-6
lines changed

‎src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationExtensions.cs

+4-6
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ public static IConfigurationBuilder SetFileProvider(this IConfigurationBuilder b
2929
return builder;
3030
}
3131

32+
internal static IFileProvider? GetUserDefinedFileProvider(this IConfigurationBuilder builder)
33+
=> builder.Properties.TryGetValue(FileProviderKey, out object? provider) ? (IFileProvider)provider : null;
34+
3235
/// <summary>
3336
/// Gets the default <see cref="IFileProvider"/> to be used for file-based providers.
3437
/// </summary>
@@ -38,12 +41,7 @@ public static IFileProvider GetFileProvider(this IConfigurationBuilder builder)
3841
{
3942
ThrowHelper.ThrowIfNull(builder);
4043

41-
if (builder.Properties.TryGetValue(FileProviderKey, out object? provider))
42-
{
43-
return (IFileProvider)provider;
44-
}
45-
46-
return new PhysicalFileProvider(AppContext.BaseDirectory ?? string.Empty);
44+
return GetUserDefinedFileProvider(builder) ?? new PhysicalFileProvider(AppContext.BaseDirectory ?? string.Empty);
4745
}
4846

4947
/// <summary>

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

+5
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ private void HandleException(ExceptionDispatchInfo info)
162162
protected virtual void Dispose(bool disposing)
163163
{
164164
_changeTokenRegistration?.Dispose();
165+
166+
if (Source.OwnsFileProvider)
167+
{
168+
(Source.FileProvider as IDisposable)?.Dispose();
169+
}
165170
}
166171
}
167172
}

‎src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs

+11
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ public abstract class FileConfigurationSource : IConfigurationSource
1818
/// </summary>
1919
public IFileProvider? FileProvider { get; set; }
2020

21+
/// <summary>
22+
/// Set to true when <see cref="FileProvider"/> was not provided by user and can be safely disposed.
23+
/// </summary>
24+
internal bool OwnsFileProvider { get; private set; }
25+
2126
/// <summary>
2227
/// The path to the file.
2328
/// </summary>
@@ -58,6 +63,11 @@ public abstract class FileConfigurationSource : IConfigurationSource
5863
/// <param name="builder">The <see cref="IConfigurationBuilder"/>.</param>
5964
public void EnsureDefaults(IConfigurationBuilder builder)
6065
{
66+
if (FileProvider is null && builder.GetUserDefinedFileProvider() is null)
67+
{
68+
OwnsFileProvider = true;
69+
}
70+
6171
FileProvider ??= builder.GetFileProvider();
6272
OnLoadException ??= builder.GetFileLoadExceptionHandler();
6373
}
@@ -81,6 +91,7 @@ public void ResolveFileProvider()
8191
}
8292
if (Directory.Exists(directory))
8393
{
94+
OwnsFileProvider = true;
8495
FileProvider = new PhysicalFileProvider(directory);
8596
Path = pathToFile;
8697
}

‎src/libraries/Microsoft.Extensions.Configuration.Json/tests/JsonConfigurationTest.cs

+61
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
using System;
55
using System.Globalization;
66
using System.IO;
7+
using System.Linq;
78
using Microsoft.Extensions.Configuration.Json;
89
using Microsoft.Extensions.Configuration.Test;
10+
using Microsoft.Extensions.FileProviders;
911
using Xunit;
1012

1113
namespace Microsoft.Extensions.Configuration
@@ -219,5 +221,64 @@ public void ThrowFormatExceptionWhenFileIsEmpty()
219221
var exception = Assert.Throws<FormatException>(() => LoadProvider(@""));
220222
Assert.Contains("Could not parse the JSON file.", exception.Message);
221223
}
224+
225+
[Theory]
226+
[InlineData(true)]
227+
[InlineData(false)]
228+
public void AddJsonFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User(bool disposeConfigRoot)
229+
{
230+
string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddJsonFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User)}.json");
231+
File.WriteAllText(filePath, @"{ ""some"": ""value"" }");
232+
233+
IConfigurationRoot config = new ConfigurationBuilder().AddJsonFile(filePath, optional: false).Build();
234+
JsonConfigurationProvider jsonConfigurationProvider = config.Providers.OfType<JsonConfigurationProvider>().Single();
235+
236+
Assert.NotNull(jsonConfigurationProvider.Source.FileProvider);
237+
PhysicalFileProvider fileProvider = (PhysicalFileProvider)jsonConfigurationProvider.Source.FileProvider;
238+
Assert.False(GetIsDisposed(fileProvider));
239+
240+
if (disposeConfigRoot)
241+
{
242+
(config as IDisposable).Dispose(); // disposing ConfigurationRoot
243+
}
244+
else
245+
{
246+
jsonConfigurationProvider.Dispose(); // disposing JsonConfigurationProvider
247+
}
248+
249+
Assert.True(GetIsDisposed(fileProvider));
250+
}
251+
252+
[Fact]
253+
public void AddJsonFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User()
254+
{
255+
string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddJsonFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User)}.json");
256+
File.WriteAllText(filePath, @"{ ""some"": ""value"" }");
257+
258+
PhysicalFileProvider fileProvider = new(Path.GetDirectoryName(filePath));
259+
JsonConfigurationProvider configurationProvider = new(new JsonConfigurationSource()
260+
{
261+
Path = filePath,
262+
FileProvider = fileProvider
263+
});
264+
IConfigurationRoot config = new ConfigurationBuilder().AddJsonFile(configurationProvider.Source.FileProvider, filePath, optional: true, reloadOnChange: false).Build();
265+
266+
Assert.False(GetIsDisposed(fileProvider));
267+
268+
(config as IDisposable).Dispose(); // disposing ConfigurationRoot that does not own the provider
269+
Assert.False(GetIsDisposed(fileProvider));
270+
271+
configurationProvider.Dispose(); // disposing JsonConfigurationProvider that does not own the provider
272+
Assert.False(GetIsDisposed(fileProvider));
273+
274+
fileProvider.Dispose(); // disposing PhysicalFileProvider itself
275+
Assert.True(GetIsDisposed(fileProvider));
276+
}
277+
278+
private static bool GetIsDisposed(PhysicalFileProvider fileProvider)
279+
{
280+
System.Reflection.FieldInfo isDisposedField = typeof(PhysicalFileProvider).GetField("_disposed", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic);
281+
return (bool)isDisposedField.GetValue(fileProvider);
282+
}
222283
}
223284
}

‎src/libraries/Microsoft.Extensions.Configuration.Xml/tests/XmlConfigurationTest.cs

+61
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33

44
using System;
55
using System.IO;
6+
using System.Linq;
67
using System.Security.Cryptography;
78
using System.Security.Cryptography.Xml;
89
using System.Tests;
910
using System.Xml;
1011
using Microsoft.Extensions.Configuration.Test;
12+
using Microsoft.Extensions.FileProviders;
1113
using Xunit;
1214

1315
namespace Microsoft.Extensions.Configuration.Xml.Test
@@ -779,5 +781,64 @@ public void LoadKeyValuePairsFromValidEncryptedXml()
779781
Assert.Equal("AnotherTestConnectionString", xmlConfigSrc.Get("data.setting:inventory:connectionstring"));
780782
Assert.Equal("MySql", xmlConfigSrc.Get("Data.setting:Inventory:Provider"));
781783
}
784+
785+
[Theory]
786+
[InlineData(true)]
787+
[InlineData(false)]
788+
public void AddXmlFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User(bool disposeConfigRoot)
789+
{
790+
string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddXmlFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User)}.xml");
791+
File.WriteAllText(filePath, @"<settings><My><Nice>Settings</Nice></My></settings>");
792+
793+
IConfigurationRoot config = new ConfigurationBuilder().AddXmlFile(filePath, optional: false).Build();
794+
XmlConfigurationProvider xmlConfigurationProvider = config.Providers.OfType<XmlConfigurationProvider>().Single();
795+
796+
Assert.NotNull(xmlConfigurationProvider.Source.FileProvider);
797+
PhysicalFileProvider fileProvider = (PhysicalFileProvider)xmlConfigurationProvider.Source.FileProvider;
798+
Assert.False(GetIsDisposed(fileProvider));
799+
800+
if (disposeConfigRoot)
801+
{
802+
(config as IDisposable).Dispose(); // disposing ConfigurationRoot
803+
}
804+
else
805+
{
806+
xmlConfigurationProvider.Dispose(); // disposing XmlConfigurationProvider
807+
}
808+
809+
Assert.True(GetIsDisposed(fileProvider));
810+
}
811+
812+
[Fact]
813+
public void AddXmlFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User()
814+
{
815+
string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddXmlFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User)}.xml");
816+
File.WriteAllText(filePath, @"<settings><My><Nice>Settings</Nice></My></settings>");
817+
818+
PhysicalFileProvider fileProvider = new(Path.GetDirectoryName(filePath));
819+
XmlConfigurationProvider configurationProvider = new(new XmlConfigurationSource()
820+
{
821+
Path = filePath,
822+
FileProvider = fileProvider
823+
});
824+
IConfigurationRoot config = new ConfigurationBuilder().AddXmlFile(configurationProvider.Source.FileProvider, filePath, optional: true, reloadOnChange: false).Build();
825+
826+
Assert.False(GetIsDisposed(fileProvider));
827+
828+
(config as IDisposable).Dispose(); // disposing ConfigurationRoot that does not own the provider
829+
Assert.False(GetIsDisposed(fileProvider));
830+
831+
configurationProvider.Dispose(); // disposing XmlConfigurationProvider
832+
Assert.False(GetIsDisposed(fileProvider));
833+
834+
fileProvider.Dispose(); // disposing PhysicalFileProvider itself
835+
Assert.True(GetIsDisposed(fileProvider));
836+
}
837+
838+
private static bool GetIsDisposed(PhysicalFileProvider fileProvider)
839+
{
840+
System.Reflection.FieldInfo isDisposedField = typeof(PhysicalFileProvider).GetField("_disposed", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic);
841+
return (bool)isDisposedField.GetValue(fileProvider);
842+
}
782843
}
783844
}

0 commit comments

Comments
 (0)