Skip to content

Commit 049725b

Browse files
adamsitnikmichaelgsharp
adamsitnik
authored and
michaelgsharp
committed
Revert "FileConfigurationProvider.Dispose should dispose FileProvider when it owns it" (dotnet#101609)
* Revert "FileConfigurationProvider.Dispose should dispose FileProvider when it…" This reverts commit 63fad3c. * Add test to ensure the bug does not come back
1 parent 06634ee commit 049725b

File tree

5 files changed

+15
-120
lines changed

5 files changed

+15
-120
lines changed

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

+6-4
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ 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-
3532
/// <summary>
3633
/// Gets the default <see cref="IFileProvider"/> to be used for file-based providers.
3734
/// </summary>
@@ -41,7 +38,12 @@ public static IFileProvider GetFileProvider(this IConfigurationBuilder builder)
4138
{
4239
ThrowHelper.ThrowIfNull(builder);
4340

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

4749
/// <summary>

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

-5
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,6 @@ 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-
}
170165
}
171166
}
172167
}

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

-11
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@ 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-
2621
/// <summary>
2722
/// The path to the file.
2823
/// </summary>
@@ -63,11 +58,6 @@ public abstract class FileConfigurationSource : IConfigurationSource
6358
/// <param name="builder">The <see cref="IConfigurationBuilder"/>.</param>
6459
public void EnsureDefaults(IConfigurationBuilder builder)
6560
{
66-
if (FileProvider is null && builder.GetUserDefinedFileProvider() is null)
67-
{
68-
OwnsFileProvider = true;
69-
}
70-
7161
FileProvider ??= builder.GetFileProvider();
7262
OnLoadException ??= builder.GetFileLoadExceptionHandler();
7363
}
@@ -91,7 +81,6 @@ public void ResolveFileProvider()
9181
}
9282
if (Directory.Exists(directory))
9383
{
94-
OwnsFileProvider = true;
9584
FileProvider = new PhysicalFileProvider(directory);
9685
Path = pathToFile;
9786
}

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

+9-39
Original file line numberDiff line numberDiff line change
@@ -222,56 +222,26 @@ public void ThrowFormatExceptionWhenFileIsEmpty()
222222
Assert.Contains("Could not parse the JSON file.", exception.Message);
223223
}
224224

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)
225+
[Fact]
226+
public void AddJsonFile_FileProvider_Is_Not_Disposed_When_SourcesGetReloaded()
229227
{
230-
string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddJsonFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User)}.json");
228+
string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddJsonFile_FileProvider_Is_Not_Disposed_When_SourcesGetReloaded)}.json");
231229
File.WriteAllText(filePath, @"{ ""some"": ""value"" }");
232230

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-
}
231+
IConfigurationBuilder builder = new ConfigurationManager();
251232

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"" }");
233+
builder.AddJsonFile(filePath, optional: false);
257234

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();
235+
FileConfigurationSource fileConfigurationSource = (FileConfigurationSource)builder.Sources.Last();
236+
PhysicalFileProvider fileProvider = (PhysicalFileProvider)fileConfigurationSource.FileProvider;
265237

266238
Assert.False(GetIsDisposed(fileProvider));
267239

268-
(config as IDisposable).Dispose(); // disposing ConfigurationRoot that does not own the provider
269-
Assert.False(GetIsDisposed(fileProvider));
240+
builder.Properties.Add("simplest", "repro");
270241

271-
configurationProvider.Dispose(); // disposing JsonConfigurationProvider that does not own the provider
272242
Assert.False(GetIsDisposed(fileProvider));
273243

274-
fileProvider.Dispose(); // disposing PhysicalFileProvider itself
244+
fileProvider.Dispose();
275245
Assert.True(GetIsDisposed(fileProvider));
276246
}
277247

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

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

44
using System;
55
using System.IO;
6-
using System.Linq;
76
using System.Security.Cryptography;
87
using System.Security.Cryptography.Xml;
98
using System.Tests;
109
using System.Xml;
1110
using Microsoft.Extensions.Configuration.Test;
12-
using Microsoft.Extensions.FileProviders;
1311
using Xunit;
1412

1513
namespace Microsoft.Extensions.Configuration.Xml.Test
@@ -781,64 +779,5 @@ public void LoadKeyValuePairsFromValidEncryptedXml()
781779
Assert.Equal("AnotherTestConnectionString", xmlConfigSrc.Get("data.setting:inventory:connectionstring"));
782780
Assert.Equal("MySql", xmlConfigSrc.Get("Data.setting:Inventory:Provider"));
783781
}
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-
}
843782
}
844783
}

0 commit comments

Comments
 (0)