Dispose configuration providers (dotnet/extensions#861) (dotnet/exten…

…sions#928)

- Dispose disposable configuration providers and their change token
  registrations when disposing the configuration root.
- Dispose the change token registration when disposing a FileConfigurationProvider.


Commit migrated from dotnet/extensions@95577f5

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

@@ -13,8 +13,10 @@ namespace Microsoft.Extensions.Configuration
     /// <summary>
     /// Base class for file based <see cref="ConfigurationProvider"/>.
     /// </summary>
-    public abstract class FileConfigurationProvider : ConfigurationProvider
+    public abstract class FileConfigurationProvider : ConfigurationProvider, IDisposable
     {
+        private readonly IDisposable _changeTokenRegistration;
+
         /// <summary>
         /// Initializes a new instance with the specified source.
         /// </summary>
@@ -29,7 +31,7 @@ public FileConfigurationProvider(FileConfigurationSource source)
 
             if (Source.ReloadOnChange && Source.FileProvider != null)
             {
-                ChangeToken.OnChange(
+                _changeTokenRegistration = ChangeToken.OnChange(
                     () => Source.FileProvider.Watch(Source.Path),
                     () => {
                         Thread.Sleep(Source.ReloadDelay);
@@ -126,5 +128,17 @@ private void HandleException(Exception e)
                 throw e;
             }
         }
+
+        /// <inheritdoc />
+        public void Dispose() => Dispose(true);
+
+        /// <summary>
+        /// Dispose the provider.
+        /// </summary>
+        /// <param name="disposing"><c>true</c> if invoked from <see cref="IDisposable.Dispose"/>.</param>
+        protected virtual void Dispose(bool disposing)
+        {
+            _changeTokenRegistration?.Dispose();
+        }
     }
 }

src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationBuilderExtensionsTest.cs

@@ -6,7 +6,7 @@
 using Microsoft.Extensions.FileProviders;
 using Xunit;
 
-namespace Microsoft.Extensions.Configuration.Json
+namespace Microsoft.Extensions.Configuration.FileExtensions.Test
 {
     public class FileConfigurationBuilderExtensionsTest
     {

src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationProviderTest.cs

@@ -0,0 +1,56 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using Microsoft.Extensions.Configuration.Test;
+using Microsoft.Extensions.FileProviders;
+using Microsoft.Extensions.Primitives;
+using Moq;
+using Xunit;
+
+namespace Microsoft.Extensions.Configuration.FileExtensions.Test
+{
+    public class FileConfigurationProviderTest
+    {
+        [Fact]
+        public void ProviderDisposesChangeTokenRegistration()
+        {
+            var changeToken = new ConfigurationRootTest.ChangeToken();
+            var fileProviderMock = new Mock<IFileProvider>();
+            fileProviderMock.Setup(fp => fp.Watch(It.IsAny<string>())).Returns(changeToken);
+
+            var provider = new FileConfigurationProviderImpl(new FileConfigurationSourceImpl
+            {
+                FileProvider = fileProviderMock.Object,
+                ReloadOnChange = true,
+            });
+
+            Assert.NotEmpty(changeToken.Callbacks);
+
+            provider.Dispose();
+
+            Assert.Empty(changeToken.Callbacks);
+        }
+
+        public class FileConfigurationProviderImpl : FileConfigurationProvider
+        {
+            public FileConfigurationProviderImpl(FileConfigurationSource source)
+                : base(source)
+            { }
+
+            public override void Load(Stream stream)
+            { }
+        }
+
+        public class FileConfigurationSourceImpl : FileConfigurationSource
+        {
+            public override IConfigurationProvider Build(IConfigurationBuilder builder)
+            {
+                EnsureDefaults(builder);
+                return new FileConfigurationProviderImpl(this);
+            }
+        }
+    }
+}

src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/Microsoft.Extensions.Configuration.FileExtensions.Tests.csproj

@@ -4,6 +4,10 @@
     <TargetFrameworks>netcoreapp3.0;net472</TargetFrameworks>
   </PropertyGroup>
 
+  <ItemGroup>
+    <ProjectReference Include="..\..\Config\test\Microsoft.Extensions.Configuration.Tests.csproj" />
+  </ItemGroup>
+
   <ItemGroup>
     <Reference Include="Microsoft.Extensions.Configuration.FileExtensions" />
   </ItemGroup>

src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationRoot.cs

@@ -12,9 +12,10 @@ namespace Microsoft.Extensions.Configuration
     /// <summary>
     /// The root node for a configuration.
     /// </summary>
-    public class ConfigurationRoot : IConfigurationRoot
+    public class ConfigurationRoot : IConfigurationRoot, IDisposable
     {
         private readonly IList<IConfigurationProvider> _providers;
+        private readonly IList<IDisposable> _changeTokenRegistrations;
         private ConfigurationReloadToken _changeToken = new ConfigurationReloadToken();
 
         /// <summary>
@@ -29,10 +30,11 @@ public ConfigurationRoot(IList<IConfigurationProvider> providers)
             }
 
             _providers = providers;
+            _changeTokenRegistrations = new List<IDisposable>(providers.Count);
             foreach (var p in providers)
             {
                 p.Load();
-                ChangeToken.OnChange(() => p.GetReloadToken(), () => RaiseChanged());
+                _changeTokenRegistrations.Add(ChangeToken.OnChange(() => p.GetReloadToken(), () => RaiseChanged()));
             }
         }
 
@@ -115,5 +117,21 @@ private void RaiseChanged()
             var previousToken = Interlocked.Exchange(ref _changeToken, new ConfigurationReloadToken());
             previousToken.OnReload();
         }
+
+        /// <inheritdoc />
+        public void Dispose()
+        {
+            // dispose change token registrations
+            foreach (var registration in _changeTokenRegistrations)
+            {
+                registration.Dispose();
+            }
+
+            // dispose providers
+            foreach (var provider in _providers)
+            {
+                (provider as IDisposable)?.Dispose();
+            }
+        }
     }
 }

src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationRootTest.cs

@@ -0,0 +1,111 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using Microsoft.Extensions.Primitives;
+using Moq;
+using Xunit;
+
+namespace Microsoft.Extensions.Configuration.Test
+{
+    public class ConfigurationRootTest
+    {
+        [Fact]
+        public void RootDisposesProviders()
+        {
+            var provider1 = new TestConfigurationProvider("foo", "foo-value");
+            var provider2 = new DisposableTestConfigurationProvider("bar", "bar-value");
+            var provider3 = new TestConfigurationProvider("baz", "baz-value");
+            var provider4 = new DisposableTestConfigurationProvider("qux", "qux-value");
+            var provider5 = new DisposableTestConfigurationProvider("quux", "quux-value");
+
+            var config = new ConfigurationRoot(new IConfigurationProvider[] {
+                provider1, provider2, provider3, provider4, provider5
+            });
+
+            Assert.Equal("foo-value", config["foo"]);
+            Assert.Equal("bar-value", config["bar"]);
+            Assert.Equal("baz-value", config["baz"]);
+            Assert.Equal("qux-value", config["qux"]);
+            Assert.Equal("quux-value", config["quux"]);
+
+            config.Dispose();
+
+            Assert.True(provider2.IsDisposed);
+            Assert.True(provider4.IsDisposed);
+            Assert.True(provider5.IsDisposed);
+        }
+
+        [Fact]
+        public void RootDisposesChangeTokenRegistrations()
+        {
+            var changeToken = new ChangeToken();
+            var providerMock = new Mock<IConfigurationProvider>();
+            providerMock.Setup(p => p.GetReloadToken()).Returns(changeToken);
+
+            var config = new ConfigurationRoot(new IConfigurationProvider[] {
+                providerMock.Object,
+            });
+
+            Assert.NotEmpty(changeToken.Callbacks);
+
+            config.Dispose();
+
+            Assert.Empty(changeToken.Callbacks);
+        }
+
+        private class TestConfigurationProvider : ConfigurationProvider
+        {
+            public TestConfigurationProvider(string key, string value)
+                => Data.Add(key, value);
+        }
+
+        private class DisposableTestConfigurationProvider : ConfigurationProvider, IDisposable
+        {
+            public bool IsDisposed { get; set; }
+
+            public DisposableTestConfigurationProvider(string key, string value)
+                => Data.Add(key, value);
+
+            public void Dispose()
+                => IsDisposed = true;
+        }
+
+        public class ChangeToken : IChangeToken
+        {
+            public List<(Action<object>, object)> Callbacks { get; } = new List<(Action<object>, object)>();
+
+            public bool HasChanged => false;
+
+            public bool ActiveChangeCallbacks => true;
+
+            public IDisposable RegisterChangeCallback(Action<object> callback, object state)
+            {
+                var item = (callback, state);
+                Callbacks.Add(item);
+                return new DisposableAction(() => Callbacks.Remove(item));
+            }
+
+            private class DisposableAction : IDisposable
+            {
+                private Action _action;
+
+                public DisposableAction(Action action)
+                {
+                    _action = action;
+                }
+
+                public void Dispose()
+                {
+                    var a = _action;
+                    if (a != null)
+                    {
+                        _action = null;
+                        a();
+                    }
+                }
+            }
+        }
+    }
+}