Skip to content

Commit 603061a

Browse files
Copilotisourabh
Copilot
and
isourabh
authored
Tolerate concurrent access to configuration files
Co-authored-by: isourabh <2982389+isourabh@users.noreply.github.com>
1 parent 6992a5a commit 603061a

3 files changed

Lines changed: 140 additions & 4 deletions

File tree

‎MSStore.CLI.UnitTests/ConfigurationManagerUnitTests.cs‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using MSStore.CLI.Services;
5+
using MSStore.CLI.Services.Telemetry;
6+
7+
namespace MSStore.CLI.UnitTests
8+
{
9+
[TestClass]
10+
public class ConfigurationManagerUnitTests
11+
{
12+
private ConfigurationManager<TelemetryConfigurations> _configurationManager = null!;
13+
14+
public TestContext TestContext { get; set; } = null!;
15+
16+
[TestInitialize]
17+
public void Initialize()
18+
{
19+
_configurationManager = new ConfigurationManager<TelemetryConfigurations>(
20+
TelemetrySourceGenerationContext.Default.TelemetryConfigurations,
21+
$"test_{Guid.NewGuid()}.json",
22+
null);
23+
}
24+
25+
[TestCleanup]
26+
public void Cleanup()
27+
{
28+
if (File.Exists(_configurationManager.ConfigPath))
29+
{
30+
File.Delete(_configurationManager.ConfigPath);
31+
}
32+
}
33+
34+
[TestMethod]
35+
public async Task SaveAsyncWaitsForOtherProcessToReleaseTheFile()
36+
{
37+
await _configurationManager.ClearAsync(TestContext.CancellationToken);
38+
39+
var otherProcessFile = File.Open(_configurationManager.ConfigPath, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
40+
41+
var saveTask = _configurationManager.SaveAsync(new TelemetryConfigurations { TelemetryEnabled = true }, TestContext.CancellationToken);
42+
43+
await Task.Delay(100, TestContext.CancellationToken);
44+
45+
otherProcessFile.Dispose();
46+
47+
await saveTask;
48+
49+
var telemetryConfigurations = await _configurationManager.LoadAsync(true, TestContext.CancellationToken);
50+
51+
telemetryConfigurations.TelemetryEnabled.Should().BeTrue();
52+
}
53+
54+
[TestMethod]
55+
public async Task LoadAsyncDoesNotThrowIfFileIsLockedByAnotherProcess()
56+
{
57+
await _configurationManager.ClearAsync(TestContext.CancellationToken);
58+
await _configurationManager.SaveAsync(new TelemetryConfigurations { TelemetryEnabled = true }, TestContext.CancellationToken);
59+
60+
using var otherProcessFile = File.Open(_configurationManager.ConfigPath, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
61+
62+
var telemetryConfigurations = await _configurationManager.LoadAsync(true, TestContext.CancellationToken);
63+
64+
telemetryConfigurations.Should().NotBeNull();
65+
66+
// The file of the other process should not have been cleared.
67+
otherProcessFile.Length.Should().BeGreaterThan(0);
68+
}
69+
70+
[TestMethod]
71+
public async Task ConcurrentLoadsAndSavesDoNotThrow()
72+
{
73+
var tasks = new List<Task>();
74+
75+
for (var i = 0; i < 5; i++)
76+
{
77+
tasks.Add(Task.Run(
78+
async () =>
79+
{
80+
var telemetryConfigurations = await _configurationManager.LoadAsync(true, TestContext.CancellationToken);
81+
telemetryConfigurations.TelemetryGuid = Guid.NewGuid().ToString();
82+
await _configurationManager.SaveAsync(telemetryConfigurations, TestContext.CancellationToken);
83+
},
84+
TestContext.CancellationToken));
85+
}
86+
87+
var act = () => Task.WhenAll(tasks);
88+
89+
await act.Should().NotThrowAsync();
90+
}
91+
}
92+
}

‎MSStore.CLI/Program.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.CommandLine.Invocation;
7+
using System.IO;
78
using System.Linq;
89
using System.Net.Http;
910
using System.Net.Http.Headers;
@@ -274,7 +275,15 @@ private static async Task<TelemetryClient> CreateTelemetryClientAsync(Configurat
274275

275276
if (changed)
276277
{
277-
await telemetryConfigurationManager.SaveAsync(telemetryConfigurations, CancellationToken.None);
278+
try
279+
{
280+
await telemetryConfigurationManager.SaveAsync(telemetryConfigurations, CancellationToken.None);
281+
}
282+
catch (IOException)
283+
{
284+
// Telemetry settings are incidental bookkeeping. If another instance of the CLI
285+
// is using the file, just move on instead of failing the command.
286+
}
278287
}
279288

280289
TelemetryConfiguration telemetryConfiguration = TelemetryConfiguration.CreateDefault();

‎MSStore.CLI/Services/ConfigurationManager.cs‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ namespace MSStore.CLI.Services
1919
internal class ConfigurationManager<T>(JsonTypeInfo<T> jsonTypeInfo, string fileName, ILogger<ConfigurationManager<T>>? logger) : IConfigurationManager<T>
2020
where T : new()
2121
{
22+
private const int MaxOpenAttempts = 5;
23+
2224
private static readonly string SettingsDirectory = Path.Combine(GetSystemLocalApplicationDataPath(), "Microsoft", "MSStore.CLI");
2325

2426
private static string GetSystemLocalApplicationDataPath()
@@ -43,6 +45,8 @@ private static string GetSystemLocalApplicationDataPath()
4345
return Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
4446
}
4547

48+
private static readonly TimeSpan OpenRetryDelay = TimeSpan.FromMilliseconds(50);
49+
4650
private readonly string _settingsPath = Path.Combine(SettingsDirectory, fileName);
4751
private readonly JsonTypeInfo<T> _jsonTypeInfo = jsonTypeInfo ?? throw new ArgumentNullException(nameof(jsonTypeInfo));
4852
private readonly ILogger? _logger = logger;
@@ -59,10 +63,23 @@ public async Task<T> LoadAsync(bool clearInvalidConfig, CancellationToken ct)
5963
return await ClearAsync(ct);
6064
}
6165

62-
using var file = File.Open(_settingsPath, FileMode.Open);
66+
using var file = await OpenAsync(FileMode.Open, ct);
6367

6468
return await JsonSerializer.DeserializeAsync(file, _jsonTypeInfo, ct) ?? new T();
6569
}
70+
catch (IOException ex)
71+
{
72+
// Another process is using the file. Do not overwrite its contents,
73+
// just fallback to the default configuration.
74+
_logger?.LogWarning(ex, "Could not read the configuration file: {SettingsPath}", _settingsPath);
75+
76+
if (!clearInvalidConfig)
77+
{
78+
throw;
79+
}
80+
81+
return new T();
82+
}
6683
catch
6784
{
6885
if (!clearInvalidConfig)
@@ -77,7 +94,7 @@ public async Task<T> LoadAsync(bool clearInvalidConfig, CancellationToken ct)
7794
public async Task<T> ClearAsync(CancellationToken ct)
7895
{
7996
EnsureDirectoryExists();
80-
using var file = File.Open(_settingsPath, FileMode.OpenOrCreate);
97+
using var file = await OpenAsync(FileMode.OpenOrCreate, ct);
8198
file.SetLength(0);
8299
await file.FlushAsync(ct);
83100
file.Position = 0;
@@ -88,12 +105,30 @@ public async Task<T> ClearAsync(CancellationToken ct)
88105

89106
public async Task SaveAsync(T config, CancellationToken ct)
90107
{
91-
using var file = File.Open(_settingsPath, FileMode.OpenOrCreate);
108+
using var file = await OpenAsync(FileMode.OpenOrCreate, ct);
92109
file.SetLength(0);
93110
file.Position = 0;
94111
await JsonSerializer.SerializeAsync(file, config, _jsonTypeInfo, ct);
95112
}
96113

114+
private async Task<FileStream> OpenAsync(FileMode fileMode, CancellationToken ct)
115+
{
116+
for (var attempt = 1; ; attempt++)
117+
{
118+
try
119+
{
120+
return File.Open(_settingsPath, fileMode, FileAccess.ReadWrite, FileShare.None);
121+
}
122+
catch (IOException ex) when (attempt < MaxOpenAttempts && ex is not FileNotFoundException and not DirectoryNotFoundException)
123+
{
124+
// The file is being used by another process. Wait a bit and try again.
125+
_logger?.LogInformation("Configuration file '{SettingsPath}' is in use. Retrying ({Attempt}/{MaxOpenAttempts})...", _settingsPath, attempt, MaxOpenAttempts);
126+
127+
await Task.Delay(OpenRetryDelay * attempt, ct);
128+
}
129+
}
130+
}
131+
97132
private void EnsureDirectoryExists()
98133
{
99134
if (Directory.Exists(SettingsDirectory))

0 commit comments

Comments
 (0)