From 7f6e3210ca435a716de2d624529f1ab395f5c506 Mon Sep 17 00:00:00 2001 From: Martin Costello Date: Mon, 4 Mar 2019 18:14:16 +0000 Subject: [PATCH] Atomically swap config data (dotnet/Extensions#1202) Swap the configuration providers' data atomically, rather than directly changing the property, so that any enumeration of the dictionary running during the reload operation does not throw an InvalidOperationException due to the collection being modified. Relates to dotnet/Extensions#1189. Commit migrated from https://github.com/dotnet/Extensions/commit/192abfdf3e73106e40d7651eecfb621e4f78c344 --- ...vironmentVariablesConfigurationProvider.cs | 6 +- .../tests/EnvironmentVariablesTest.cs | 47 +++++++++++++++ .../FunctionalTests/ConfigurationTests.cs | 59 ++++++++++++++++++- ...sions.Configuration.FunctionalTests.csproj | 3 + 4 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/src/EnvironmentVariablesConfigurationProvider.cs b/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/src/EnvironmentVariablesConfigurationProvider.cs index 3089093ff16fa..108c639486af4 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/src/EnvironmentVariablesConfigurationProvider.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/src/EnvironmentVariablesConfigurationProvider.cs @@ -48,7 +48,7 @@ public override void Load() internal void Load(IDictionary envVariables) { - Data = new Dictionary(StringComparer.OrdinalIgnoreCase); + var data = new Dictionary(StringComparer.OrdinalIgnoreCase); var filteredEnvVariables = envVariables .Cast() @@ -58,8 +58,10 @@ internal void Load(IDictionary envVariables) foreach (var envVariable in filteredEnvVariables) { var key = ((string)envVariable.Key).Substring(_prefix.Length); - Data[key] = (string)envVariable.Value; + data[key] = (string)envVariable.Value; } + + Data = data; } private static string NormalizeKey(string key) diff --git a/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/tests/EnvironmentVariablesTest.cs b/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/tests/EnvironmentVariablesTest.cs index ca282e354796b..72504a552adca 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/tests/EnvironmentVariablesTest.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/tests/EnvironmentVariablesTest.cs @@ -1,7 +1,11 @@ // 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; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.Configuration.Test; using Xunit; @@ -148,5 +152,48 @@ public void ReplaceDoubleUnderscoreInEnvironmentVariables() Assert.Equal("connection", envConfigSrc.Get("data:ConnectionString")); Assert.Equal("System.Data.SqlClient", envConfigSrc.Get("ConnectionStrings:_db1_ProviderName")); } + + [Fact] + public void BindingDoesNotThrowIfReloadedDuringBinding() + { + var dic = new Dictionary + { + {"Number", "-2"}, + {"Text", "Foo"} + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + configurationBuilder.AddEnvironmentVariables(); + var config = configurationBuilder.Build(); + + MyOptions options = null; + + using (var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(250))) + { + void ReloadLoop() + { + while (!cts.IsCancellationRequested) + { + config.Reload(); + } + } + + _ = Task.Run(ReloadLoop); + + while (!cts.IsCancellationRequested) + { + options = config.Get(); + } + } + + Assert.Equal(-2, options.Number); + Assert.Equal("Foo", options.Text); + } + + private sealed class MyOptions + { + public int Number { get; set; } + public string Text { get; set; } + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs index 27c694c43be7c..95c90ed5caa4e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs @@ -5,13 +5,13 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Testing; using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.Configuration.Ini; using Microsoft.Extensions.Configuration.Json; -using Microsoft.Extensions.Configuration.Xml; using Microsoft.Extensions.Configuration.UserSecrets; +using Microsoft.Extensions.Configuration.Xml; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Primitives; using Xunit; @@ -944,6 +944,48 @@ public async Task TouchingFileWillReloadForUserSecrets() Assert.True(token.HasChanged); } + [Fact] + public void BindingDoesNotThrowIfReloadedDuringBinding() + { + WriteTestFiles(); + + var configurationBuilder = CreateBuilder(); + configurationBuilder.Add(new TestIniSourceProvider(_iniFile)); + configurationBuilder.Add(new TestJsonSourceProvider(_jsonFile)); + configurationBuilder.Add(new TestXmlSourceProvider(_xmlFile)); + configurationBuilder.AddEnvironmentVariables(); + configurationBuilder.AddCommandLine(new[] { "--CmdKey1=CmdValue1" }); + configurationBuilder.AddInMemoryCollection(_memConfigContent); + + var config = configurationBuilder.Build(); + + using (var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(250))) + { + void ReloadLoop() + { + while (!cts.IsCancellationRequested) + { + config.Reload(); + } + } + + _ = Task.Run(ReloadLoop); + + MyOptions options = null; + + while (!cts.IsCancellationRequested) + { + options = config.Get(); + } + + Assert.Equal("CmdValue1", options.CmdKey1); + Assert.Equal("IniValue1", options.IniKey1); + Assert.Equal("JsonValue1", options.JsonKey1); + Assert.Equal("MemValue1", options.MemKey1); + Assert.Equal("XmlValue1", options.XmlKey1); + } + } + public void Dispose() { _fileProvider.Dispose(); @@ -966,5 +1008,18 @@ public void Dispose() await Task.Delay(_msDelay); } } + + private sealed class MyOptions + { + public string CmdKey1 { get; set; } + + public string IniKey1 { get; set; } + + public string JsonKey1 { get; set; } + + public string MemKey1 { get; set; } + + public string XmlKey1 { get; set; } + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/Microsoft.Extensions.Configuration.FunctionalTests.csproj b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/Microsoft.Extensions.Configuration.FunctionalTests.csproj index 6f7dbe3ddd1c5..818806cf49309 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/Microsoft.Extensions.Configuration.FunctionalTests.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/Microsoft.Extensions.Configuration.FunctionalTests.csproj @@ -9,6 +9,9 @@ + + +