FileConfigurationProvider.Dispose should dispose FileProvider when it ows it (#86455)

fixes #86146

diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationExtensions.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationExtensions.cs
index f84c5c1..77ac387 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationExtensions.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationExtensions.cs
@@ -29,6 +29,9 @@ namespace Microsoft.Extensions.Configuration
             return builder;
         }
 
+        internal static IFileProvider? GetUserDefinedFileProvider(this IConfigurationBuilder builder)
+            => builder.Properties.TryGetValue(FileProviderKey, out object? provider) ? (IFileProvider)provider : null;
+
         /// <summary>
         /// Gets the default <see cref="IFileProvider"/> to be used for file-based providers.
         /// </summary>
@@ -38,12 +41,7 @@ namespace Microsoft.Extensions.Configuration
         {
             ThrowHelper.ThrowIfNull(builder);
 
-            if (builder.Properties.TryGetValue(FileProviderKey, out object? provider))
-            {
-                return (IFileProvider)provider;
-            }
-
-            return new PhysicalFileProvider(AppContext.BaseDirectory ?? string.Empty);
+            return GetUserDefinedFileProvider(builder) ?? new PhysicalFileProvider(AppContext.BaseDirectory ?? string.Empty);
         }
 
         /// <summary>

diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
index d226051..c0d8c9f 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
@@ -162,6 +162,11 @@ namespace Microsoft.Extensions.Configuration
         protected virtual void Dispose(bool disposing)
         {
             _changeTokenRegistration?.Dispose();
+
+            if (Source.OwnsFileProvider)
+            {
+                (Source.FileProvider as IDisposable)?.Dispose();
+            }
         }
     }
 }

diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs
index d58c265..60555b2 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs
@@ -19,6 +19,11 @@ namespace Microsoft.Extensions.Configuration
         public IFileProvider? FileProvider { get; set; }
 
         /// <summary>
+        /// Set to true when <see cref="FileProvider"/> was not provided by user and can be safely disposed.
+        /// </summary>
+        internal bool OwnsFileProvider { get; private set; }
+
+        /// <summary>
         /// The path to the file.
         /// </summary>
         [DisallowNull]
@@ -58,6 +63,11 @@ namespace Microsoft.Extensions.Configuration
         /// <param name="builder">The <see cref="IConfigurationBuilder"/>.</param>
         public void EnsureDefaults(IConfigurationBuilder builder)
         {
+            if (FileProvider is null && builder.GetUserDefinedFileProvider() is null)
+            {
+                OwnsFileProvider = true;
+            }
+
             FileProvider ??= builder.GetFileProvider();
             OnLoadException ??= builder.GetFileLoadExceptionHandler();
         }
@@ -81,6 +91,7 @@ namespace Microsoft.Extensions.Configuration
                 }
                 if (Directory.Exists(directory))
                 {
+                    OwnsFileProvider = true;
                     FileProvider = new PhysicalFileProvider(directory);
                     Path = pathToFile;
                 }

diff --git a/src/libraries/Microsoft.Extensions.Configuration.Json/tests/JsonConfigurationTest.cs b/src/libraries/Microsoft.Extensions.Configuration.Json/tests/JsonConfigurationTest.cs
index 2e3fe00..4b08c91 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.Json/tests/JsonConfigurationTest.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.Json/tests/JsonConfigurationTest.cs
@@ -4,8 +4,10 @@
 using System;
 using System.Globalization;
 using System.IO;
+using System.Linq;
 using Microsoft.Extensions.Configuration.Json;
 using Microsoft.Extensions.Configuration.Test;
+using Microsoft.Extensions.FileProviders;
 using Xunit;
 
 namespace Microsoft.Extensions.Configuration
@@ -219,5 +221,64 @@ namespace Microsoft.Extensions.Configuration
             var exception = Assert.Throws<FormatException>(() => LoadProvider(@""));
             Assert.Contains("Could not parse the JSON file.", exception.Message);
         }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void AddJsonFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User(bool disposeConfigRoot)
+        {
+            string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddJsonFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User)}.json");
+            File.WriteAllText(filePath, @"{ ""some"": ""value"" }");
+
+            IConfigurationRoot config = new ConfigurationBuilder().AddJsonFile(filePath, optional: false).Build();
+            JsonConfigurationProvider jsonConfigurationProvider = config.Providers.OfType<JsonConfigurationProvider>().Single();
+
+            Assert.NotNull(jsonConfigurationProvider.Source.FileProvider);
+            PhysicalFileProvider fileProvider = (PhysicalFileProvider)jsonConfigurationProvider.Source.FileProvider;
+            Assert.False(GetIsDisposed(fileProvider));
+
+            if (disposeConfigRoot)
+            {
+                (config as IDisposable).Dispose(); // disposing ConfigurationRoot
+            }
+            else
+            {
+                jsonConfigurationProvider.Dispose(); // disposing JsonConfigurationProvider
+            }
+
+            Assert.True(GetIsDisposed(fileProvider));
+        }
+
+        [Fact]
+        public void AddJsonFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User()
+        {
+            string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddJsonFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User)}.json");
+            File.WriteAllText(filePath, @"{ ""some"": ""value"" }");
+
+            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();
+
+            Assert.False(GetIsDisposed(fileProvider));
+
+            (config as IDisposable).Dispose(); // disposing ConfigurationRoot that does not own the provider
+            Assert.False(GetIsDisposed(fileProvider));
+
+            configurationProvider.Dispose(); // disposing JsonConfigurationProvider that does not own the provider
+            Assert.False(GetIsDisposed(fileProvider));
+
+            fileProvider.Dispose(); // disposing PhysicalFileProvider itself
+            Assert.True(GetIsDisposed(fileProvider));
+        }
+
+        private static bool GetIsDisposed(PhysicalFileProvider fileProvider)
+        {
+            System.Reflection.FieldInfo isDisposedField = typeof(PhysicalFileProvider).GetField("_disposed", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic);
+            return (bool)isDisposedField.GetValue(fileProvider);
+        }
     }
 }

diff --git a/src/libraries/Microsoft.Extensions.Configuration.Xml/tests/XmlConfigurationTest.cs b/src/libraries/Microsoft.Extensions.Configuration.Xml/tests/XmlConfigurationTest.cs
index 4012d77..d248d96 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.Xml/tests/XmlConfigurationTest.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.Xml/tests/XmlConfigurationTest.cs
@@ -3,11 +3,13 @@
 
 using System;
 using System.IO;
+using System.Linq;
 using System.Security.Cryptography;
 using System.Security.Cryptography.Xml;
 using System.Tests;
 using System.Xml;
 using Microsoft.Extensions.Configuration.Test;
+using Microsoft.Extensions.FileProviders;
 using Xunit;
 
 namespace Microsoft.Extensions.Configuration.Xml.Test
@@ -779,5 +781,64 @@ namespace Microsoft.Extensions.Configuration.Xml.Test
             Assert.Equal("AnotherTestConnectionString", xmlConfigSrc.Get("data.setting:inventory:connectionstring"));
             Assert.Equal("MySql", xmlConfigSrc.Get("Data.setting:Inventory:Provider"));
         }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void AddXmlFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User(bool disposeConfigRoot)
+        {
+            string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddXmlFile_FileProvider_Gets_Disposed_When_It_Was_Not_Created_By_The_User)}.xml");
+            File.WriteAllText(filePath, @"<settings><My><Nice>Settings</Nice></My></settings>");
+
+            IConfigurationRoot config = new ConfigurationBuilder().AddXmlFile(filePath, optional: false).Build();
+            XmlConfigurationProvider xmlConfigurationProvider = config.Providers.OfType<XmlConfigurationProvider>().Single();
+
+            Assert.NotNull(xmlConfigurationProvider.Source.FileProvider);
+            PhysicalFileProvider fileProvider = (PhysicalFileProvider)xmlConfigurationProvider.Source.FileProvider;
+            Assert.False(GetIsDisposed(fileProvider));
+
+            if (disposeConfigRoot)
+            {
+                (config as IDisposable).Dispose(); // disposing ConfigurationRoot
+            }
+            else
+            {
+                xmlConfigurationProvider.Dispose(); // disposing XmlConfigurationProvider
+            }
+            
+            Assert.True(GetIsDisposed(fileProvider));
+        }
+
+        [Fact]
+        public void AddXmlFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User()
+        {
+            string filePath = Path.Combine(Path.GetTempPath(), $"{nameof(AddXmlFile_FileProvider_Is_Not_Disposed_When_It_Is_Owned_By_The_User)}.xml");
+            File.WriteAllText(filePath, @"<settings><My><Nice>Settings</Nice></My></settings>");
+
+            PhysicalFileProvider fileProvider = new(Path.GetDirectoryName(filePath));
+            XmlConfigurationProvider configurationProvider = new(new XmlConfigurationSource()
+            {
+                Path = filePath,
+                FileProvider = fileProvider
+            });
+            IConfigurationRoot config = new ConfigurationBuilder().AddXmlFile(configurationProvider.Source.FileProvider, filePath, optional: true, reloadOnChange: false).Build();
+
+            Assert.False(GetIsDisposed(fileProvider));
+
+            (config as IDisposable).Dispose(); // disposing ConfigurationRoot that does not own the provider
+            Assert.False(GetIsDisposed(fileProvider));
+
+            configurationProvider.Dispose(); // disposing XmlConfigurationProvider
+            Assert.False(GetIsDisposed(fileProvider));
+
+            fileProvider.Dispose(); // disposing PhysicalFileProvider itself
+            Assert.True(GetIsDisposed(fileProvider));
+        }
+
+        private static bool GetIsDisposed(PhysicalFileProvider fileProvider)
+        {
+            System.Reflection.FieldInfo isDisposedField = typeof(PhysicalFileProvider).GetField("_disposed", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic);
+            return (bool)isDisposedField.GetValue(fileProvider);
+        }
     }
 }