Never dispose providers more than once

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

@@ -9,64 +9,82 @@
 namespace Microsoft.Extensions.Configuration
 {
     // ReferenceCountedProviders is used by ConfigurationManager to wait until all readers unreference it before disposing any providers.
-    internal sealed class ReferenceCountedProviders : IDisposable
+    internal abstract class ReferenceCountedProviders : IDisposable
     {
-        private long _refCount = 1;
-        // volatile is not strictly necessary because the runtime adds a barrier either way, but volatile indicates that this field has
-        // unsynchronized readers meaning the all writes initializing the list must be published before updating the _providers reference.
-        private volatile List<IConfigurationProvider> _providers;
+        public static ReferenceCountedProviders Create(List<IConfigurationProvider> providers) => new ActiveReferenceCountedProviders(providers);
 
-        public ReferenceCountedProviders(List<IConfigurationProvider> providers)
-        {
-            _providers = providers;
-        }
+        // If anything references DisposedReferenceCountedProviders, it indicates something is using the ConfigurationManager after it's been disposed.
+        // We could preemptively throw an ODE from ReferenceCountedProviderManager.GetReference() instead of returning this type, but this might
+        // break existing apps that are previously able to continue to read configuration after disposing an ConfigurationManager.
+        public static ReferenceCountedProviders CreateDisposed(List<IConfigurationProvider> providers) => new DisposedReferenceCountedProviders(providers);
+
+        public abstract List<IConfigurationProvider> Providers { get; set; }
+        // This is only used to support IConfigurationRoot.Providers because we cannot track the lifetime of that reference.
+        public abstract List<IConfigurationProvider> NonReferenceCountedProviders { get; }
 
-        public List<IConfigurationProvider> Providers
+        public abstract void AddReference();
+        // This is Dispose() rather than RemoveReference() so we can conveniently release a reference at the end of a using block.
+        public abstract void Dispose();
+
+        private sealed class ActiveReferenceCountedProviders : ReferenceCountedProviders
         {
-            get
+            private long _refCount = 1;
+            // volatile is not strictly necessary because the runtime adds a barrier either way, but volatile indicates that this field has
+            // unsynchronized readers meaning the all writes initializing the list must be published before updating the _providers reference.
+            private volatile List<IConfigurationProvider> _providers;
+
+            public ActiveReferenceCountedProviders(List<IConfigurationProvider> providers)
             {
-                Debug.Assert(_refCount > 0);
-                return _providers;
+                _providers = providers;
             }
-            set
+
+            public override List<IConfigurationProvider> Providers
             {
-                Debug.Assert(_refCount > 0);
-                _providers = value;
+                get
+                {
+                    Debug.Assert(_refCount > 0);
+                    return _providers;
+                }
+                set
+                {
+                    Debug.Assert(_refCount > 0);
+                    _providers = value;
+                }
             }
-        }
 
-        // This is only used to support IConfigurationRoot.Providers because we cannot track the lifetime of that reference.
-        public List<IConfigurationProvider> NonReferenceCountedProviders => _providers;
+            public override List<IConfigurationProvider> NonReferenceCountedProviders => _providers;
 
-        public void AddReference()
-        {
-            // AddReference() is always called with a lock to ensure _refCount hasn't already decremented to zero.
-            Debug.Assert(_refCount > 0);
-            Interlocked.Increment(ref _refCount);
-        }
+            public override void AddReference()
+            {
+                // AddReference() is always called with a lock to ensure _refCount hasn't already decremented to zero.
+                Debug.Assert(_refCount > 0);
+                Interlocked.Increment(ref _refCount);
+            }
 
-        // This is not a "real" Dispose(). It exists to conveniently release a reference at the end of a using block.
-        public void Dispose()
-        {
-            if (Interlocked.Decrement(ref _refCount) == 0)
+            public override void Dispose()
             {
-                foreach (IConfigurationProvider provider in _providers)
+                if (Interlocked.Decrement(ref _refCount) == 0)
                 {
-                    (provider as IDisposable)?.Dispose();
+                    foreach (IConfigurationProvider provider in _providers)
+                    {
+                        (provider as IDisposable)?.Dispose();
+                    }
                 }
             }
         }
 
-        // This is the "real" Dispose() that is only called as part of ConfigurationManager.Dispose().
-        // If there are any active references, that indicates a use-after-dispose bug in the code using the ConfigurationManager.
-        //
-        // If there are active references after dispose, the providers could get disposed more than once. We could prevent this
-        // by preemptively throwing an ODE from ReferenceCountedProviderManager.GetReference() after it's disposed, but this might
-        // break existing apps that are today able to continue to read configuration after disposing an ConfigurationManager.
-        public void ReleaseFinalReference()
+        private sealed class DisposedReferenceCountedProviders : ReferenceCountedProviders
         {
-            Debug.Assert(_refCount == 1);
-            Dispose();
+            public DisposedReferenceCountedProviders(List<IConfigurationProvider> providers)
+            {
+                Providers = providers;
+            }
+
+            public override List<IConfigurationProvider> Providers { get; set; }
+            public override List<IConfigurationProvider> NonReferenceCountedProviders => Providers;
+
+            public override void AddReference() { }
+            public override void Dispose() { }
         }
     }
 }

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

@@ -12,7 +12,8 @@ namespace Microsoft.Extensions.Configuration
     internal sealed class ReferenceCountedProviderManager : IDisposable
     {
         private readonly object _replaceProvidersLock = new object();
-        private ReferenceCountedProviders _refCountedProviders = new(new List<IConfigurationProvider>());
+        private ReferenceCountedProviders _refCountedProviders = ReferenceCountedProviders.Create(new List<IConfigurationProvider>());
+        private bool _disposed;
 
         // This is only used to support IConfigurationRoot.Providers because we cannot track the lifetime of that reference.
         public IEnumerable<IConfigurationProvider> NonReferenceCountedProviders => _refCountedProviders.NonReferenceCountedProviders;
@@ -35,7 +36,12 @@ public void ReplaceProviders(List<IConfigurationProvider> providers)
 
             lock (_replaceProvidersLock)
             {
-                _refCountedProviders = new ReferenceCountedProviders(providers);
+                if (_disposed)
+                {
+                    throw new ObjectDisposedException(nameof(ConfigurationManager));
+                }
+
+                _refCountedProviders = ReferenceCountedProviders.Create(providers);
             }
 
             // Decrement the reference count to the old providers. If they are being concurrently read from
@@ -45,17 +51,36 @@ public void ReplaceProviders(List<IConfigurationProvider> providers)
 
         public void AddProvider(IConfigurationProvider provider)
         {
-            // Maintain existing references, but replace list with copy containing new item.
-            _refCountedProviders.Providers = new List<IConfigurationProvider>(_refCountedProviders.Providers)
+            lock (_replaceProvidersLock)
             {
-                provider
-            };
+                if (_disposed)
+                {
+                    throw new ObjectDisposedException(nameof(ConfigurationManager));
+                }
+
+                // Maintain existing references, but replace list with copy containing new item.
+                _refCountedProviders.Providers = new List<IConfigurationProvider>(_refCountedProviders.Providers)
+                {
+                    provider
+                };
+            }
         }
 
         public void Dispose()
         {
-            // Equivalent to _refCountedProvider.Dispose() but asserts there a no concurrent references.
-            _refCountedProviders.ReleaseFinalReference();
+            ReferenceCountedProviders oldRefCountedProviders = _refCountedProviders;
+
+            lock (_replaceProvidersLock)
+            {
+                _disposed = true;
+
+                // Create a non-reference-counting ReferenceCountedProviders instance now that the ConfigurationManager is disposed.
+                // We could preemptively throw an ODE from ReferenceCountedProviderManager.GetReference() instead, but this might
+                // break existing apps that were previously able to continue to read configuration after disposing an ConfigurationManager.
+                _refCountedProviders = ReferenceCountedProviders.CreateDisposed(oldRefCountedProviders.Providers);
+            }
+
+            oldRefCountedProviders.Dispose();
         }
     }
 }