From d66ab3d1e4f9819dc4c40a327424eb35b9cbd83c Mon Sep 17 00:00:00 2001 From: Peter Csajtai Date: Thu, 31 Mar 2022 13:02:57 +0200 Subject: [PATCH] Handle files locked by another process, ensure logger is always wrapped --- .../ConfigCatClientTests.cs | 2 + .../ConfigEvaluatorTestsBase.cs | 4 +- src/ConfigCatClient/ConfigCatClient.cs | 6 +- .../Configuration/ConfigurationBase.cs | 4 +- src/ConfigCatClient/Logging/LoggerWrapper.cs | 9 +-- .../Override/LocalFileDataSource.cs | 71 ++++++++++++------- 6 files changed, 60 insertions(+), 36 deletions(-) diff --git a/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs b/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs index a2a451a4..1d974237 100644 --- a/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs +++ b/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs @@ -27,6 +27,8 @@ public void TestInitialize() loggerMock.Reset(); evaluatorMock.Reset(); configDeserializerMock.Reset(); + + loggerMock.Setup(l => l.LogLevel).Returns(LogLevel.Warning); } [ExpectedException(typeof(ArgumentException))] diff --git a/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs b/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs index f1a8c468..50bd8cfe 100644 --- a/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs +++ b/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs @@ -11,7 +11,7 @@ namespace ConfigCat.Client.Tests { public abstract class ConfigEvaluatorTestsBase { - private readonly ILogger logger = new LoggerWrapper(new ConsoleLogger(LogLevel.Debug)); + private readonly ILogger logger = new ConsoleLogger(LogLevel.Debug); private protected readonly IDictionary config; @@ -23,7 +23,7 @@ public abstract class ConfigEvaluatorTestsBase public ConfigEvaluatorTestsBase() { - this.configEvaluator = new RolloutEvaluator(logger); + this.configEvaluator = new RolloutEvaluator(new LoggerWrapper(logger)); this.config = this.GetSampleJson().Deserialize().Settings; } diff --git a/src/ConfigCatClient/ConfigCatClient.cs b/src/ConfigCatClient/ConfigCatClient.cs index cf718cf7..5a7443bb 100644 --- a/src/ConfigCatClient/ConfigCatClient.cs +++ b/src/ConfigCatClient/ConfigCatClient.cs @@ -121,7 +121,7 @@ public ConfigCatClient(Action configurationAction) configuration.Validate(); - this.log = configuration.Logger; + this.log = new LoggerWrapper(configuration.Logger); this.configDeserializer = new ConfigDeserializer(); this.configEvaluator = new RolloutEvaluator(this.log); @@ -151,12 +151,12 @@ public ConfigCatClient(Action configurationAction) } /// - /// For test purpose only + /// For testing purposes only /// internal ConfigCatClient(IConfigService configService, ILogger logger, IRolloutEvaluator evaluator, IConfigDeserializer configDeserializer) { this.configService = configService; - this.log = logger; + this.log = new LoggerWrapper(logger); this.configEvaluator = evaluator; this.configDeserializer = configDeserializer; } diff --git a/src/ConfigCatClient/Configuration/ConfigurationBase.cs b/src/ConfigCatClient/Configuration/ConfigurationBase.cs index b4455968..8b0dee3c 100644 --- a/src/ConfigCatClient/Configuration/ConfigurationBase.cs +++ b/src/ConfigCatClient/Configuration/ConfigurationBase.cs @@ -9,7 +9,7 @@ namespace ConfigCat.Client /// public abstract class ConfigurationBase { - private ILogger logger = LoggerWrapper.Default; + private ILogger logger = new ConsoleLogger(LogLevel.Warning); /// /// Logger instance. @@ -19,7 +19,7 @@ public ILogger Logger get => this.logger; set { - this.logger = new LoggerWrapper(value ?? throw new ArgumentNullException(nameof(Logger))); + this.logger = value ?? throw new ArgumentNullException(nameof(Logger)); } } diff --git a/src/ConfigCatClient/Logging/LoggerWrapper.cs b/src/ConfigCatClient/Logging/LoggerWrapper.cs index 2f30f0e4..765dc251 100644 --- a/src/ConfigCatClient/Logging/LoggerWrapper.cs +++ b/src/ConfigCatClient/Logging/LoggerWrapper.cs @@ -4,15 +4,16 @@ namespace ConfigCat.Client { internal sealed class LoggerWrapper : ILogger { - public static readonly LoggerWrapper Default = new(new ConsoleLogger()); - private readonly ILogger logger; - public LogLevel LogLevel { get; set; } + public LogLevel LogLevel + { + get => logger.LogLevel; + set => logger.LogLevel = value; + } internal LoggerWrapper(ILogger logger) { - this.LogLevel = logger.LogLevel; this.logger = logger; } diff --git a/src/ConfigCatClient/Override/LocalFileDataSource.cs b/src/ConfigCatClient/Override/LocalFileDataSource.cs index 0c45ed67..ece7955f 100644 --- a/src/ConfigCatClient/Override/LocalFileDataSource.cs +++ b/src/ConfigCatClient/Override/LocalFileDataSource.cs @@ -10,7 +10,11 @@ namespace ConfigCat.Client.Override { internal sealed class LocalFileDataSource : IOverrideDataSource { + const int WAIT_TIME_FOR_UNLOCK = 200; // ms + const int MAX_WAIT_ITERATIONS = 50; + private int isReading; + private readonly string fullPath; private readonly ILogger logger; private readonly FileSystemWatcher fileSystemWatcher; private readonly TaskCompletionSource asyncInit = new(); @@ -20,33 +24,33 @@ internal sealed class LocalFileDataSource : IOverrideDataSource public LocalFileDataSource(string filePath, bool autoReload, ILogger logger) { + this.fullPath = Path.GetFullPath(filePath); if (autoReload) { - var directory = Path.GetDirectoryName(filePath); + var directory = Path.GetDirectoryName(this.fullPath); if (string.IsNullOrWhiteSpace(directory)) { - logger.Error($"Directory of {filePath} not found to watch."); + logger.Error($"Directory of {this.fullPath} not found to watch."); } else { - this.fileSystemWatcher = new FileSystemWatcher(directory) - { - Filter = Path.GetFileName(filePath), - NotifyFilter = NotifyFilters.LastWrite, - EnableRaisingEvents = true - }; - this.fileSystemWatcher.Changed += FileSystemWatcher_Changed; - logger.Information($"Watching {filePath} for changes."); + this.fileSystemWatcher = new FileSystemWatcher(directory); + this.fileSystemWatcher.Changed += OnChanged; + this.fileSystemWatcher.Created += OnChanged; + this.fileSystemWatcher.Renamed += OnChanged; + this.fileSystemWatcher.EnableRaisingEvents = true; + logger.Information($"Watching {this.fullPath} for changes."); } } this.logger = logger; - _ = this.ReadFileAsync(filePath); + _ = this.ReadFileAsync(this.fullPath); } - private async void FileSystemWatcher_Changed(object sender, FileSystemEventArgs e) + private async void OnChanged(object sender, FileSystemEventArgs e) { - if (e.ChangeType != WatcherChangeTypes.Changed) + // filter out events on temporary files + if (e.FullPath != this.fullPath) return; this.logger.Information($"Reload file {e.FullPath}."); @@ -74,18 +78,34 @@ private async Task ReadFileAsync(string filePath) try { - using var stream = new FileStream(filePath, FileMode.Open, FileAccess.Read); - using var reader = new StreamReader(stream); - var content = await reader.ReadToEndAsync(); - var simplified = content.DeserializeOrDefault(); - if (simplified?.Entries != null) + for (int i = 1; i <= MAX_WAIT_ITERATIONS; i++) { - this.overrideValues = simplified.Entries.ToDictionary(kv => kv.Key, kv => kv.Value.ToSetting()); - return; - } + try + { + using var stream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); + using var reader = new StreamReader(stream); + var content = await reader.ReadToEndAsync(); + var simplified = content.DeserializeOrDefault(); + if (simplified?.Entries != null) + { + this.overrideValues = simplified.Entries.ToDictionary(kv => kv.Key, kv => kv.Value.ToSetting()); + return; + } - var deserialized = content.Deserialize(); - this.overrideValues = deserialized.Settings; + var deserialized = content.Deserialize(); + this.overrideValues = deserialized.Settings; + break; + } + // this logic ensures that we keep trying to open the file for max 10s + // when it's locked by another process. + catch (IOException e) when (e.HResult == -2147024864) // ERROR_SHARING_VIOLATION + { + if (i >= MAX_WAIT_ITERATIONS) + throw; + + await Task.Delay(WAIT_TIME_FOR_UNLOCK); + } + } } catch (Exception ex) { @@ -108,10 +128,11 @@ public void Dispose() { if (this.fileSystemWatcher != null) { - this.fileSystemWatcher.Changed -= FileSystemWatcher_Changed; + this.fileSystemWatcher.Changed -= OnChanged; + this.fileSystemWatcher.Created -= OnChanged; + this.fileSystemWatcher.Renamed -= OnChanged; this.fileSystemWatcher.Dispose(); } - this.syncInit.Dispose(); }