Skip to content

Commit 9f2e20f

Browse files
Yu Leng (from Dev Box)Copilot
andcommitted
PowerDisplay: Make profile persistence atomic across processes
Serialize profile transactions across processes, publish saves atomically, and propagate persistence failures before migrating references. Also disambiguate duplicate profile names and validate profile-id IPC. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f1966791-4130-4447-94ff-09a2c3cacc30
1 parent a17cc8a commit 9f2e20f

12 files changed

Lines changed: 552 additions & 165 deletions

File tree

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
// Copyright (c) Microsoft Corporation
2+
// The Microsoft Corporation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Collections.Generic;
7+
using System.IO;
8+
using System.Linq;
9+
using System.Text.Json;
10+
using System.Threading;
11+
using System.Threading.Tasks;
12+
using Microsoft.VisualStudio.TestTools.UnitTesting;
13+
using PowerDisplay.Models;
14+
15+
namespace PowerDisplay.UnitTests;
16+
17+
[TestClass]
18+
public class ProfileStoreTests
19+
{
20+
private static readonly string[] ExpectedConcurrentProfileNames = { "First", "Second" };
21+
22+
private string _tempDir = string.Empty;
23+
private string _profilesPath = string.Empty;
24+
private string _mutexName = string.Empty;
25+
26+
[TestInitialize]
27+
public void SetUp()
28+
{
29+
_tempDir = Path.Combine(Path.GetTempPath(), $"pd-profile-store-test-{Guid.NewGuid():N}");
30+
Directory.CreateDirectory(_tempDir);
31+
_profilesPath = Path.Combine(_tempDir, "profiles.json");
32+
_mutexName = $@"Local\PowerToys_PowerDisplay_ProfileStore_Test_{Guid.NewGuid():N}";
33+
}
34+
35+
[TestCleanup]
36+
public void TearDown()
37+
{
38+
try
39+
{
40+
if (File.Exists(_profilesPath))
41+
{
42+
File.SetAttributes(_profilesPath, FileAttributes.Normal);
43+
}
44+
45+
if (Directory.Exists(_tempDir))
46+
{
47+
Directory.Delete(_tempDir, recursive: true);
48+
}
49+
}
50+
catch
51+
{
52+
// Best-effort cleanup.
53+
}
54+
}
55+
56+
[TestMethod]
57+
public void LoadProfilesEnsuringIds_CorruptJson_DoesNotOverwriteSource()
58+
{
59+
const string corruptJson = "{not-json";
60+
File.WriteAllText(_profilesPath, corruptJson);
61+
var store = CreateStore();
62+
63+
Exception? exception = null;
64+
try
65+
{
66+
store.LoadProfilesEnsuringIds();
67+
}
68+
catch (Exception ex)
69+
{
70+
exception = ex;
71+
}
72+
73+
Assert.IsInstanceOfType<JsonException>(exception);
74+
Assert.AreEqual(corruptJson, File.ReadAllText(_profilesPath));
75+
}
76+
77+
[TestMethod]
78+
public void LoadProfilesEnsuringIds_SaveFails_DoesNotReturnTransientIdsOrReplaceSource()
79+
{
80+
var profiles = new PowerDisplayProfiles();
81+
profiles.Profiles.Add(MakeProfile("Legacy"));
82+
var originalJson = JsonSerializer.Serialize(profiles, ProfileSerializationContext.Default.PowerDisplayProfiles);
83+
File.WriteAllText(_profilesPath, originalJson);
84+
File.SetAttributes(_profilesPath, FileAttributes.ReadOnly);
85+
var store = CreateStore();
86+
87+
Exception? exception = null;
88+
try
89+
{
90+
store.LoadProfilesEnsuringIds();
91+
}
92+
catch (Exception ex)
93+
{
94+
exception = ex;
95+
}
96+
97+
Assert.IsNotNull(exception);
98+
Assert.AreEqual(originalJson, File.ReadAllText(_profilesPath));
99+
Assert.AreEqual(0, store.LoadProfiles().Profiles[0].Id);
100+
Assert.IsFalse(Directory.EnumerateFiles(_tempDir, "*.tmp").Any());
101+
}
102+
103+
[TestMethod]
104+
public void AddOrUpdateProfile_WritesAtomicallyWithoutLeavingTemporaryFile()
105+
{
106+
var store = CreateStore();
107+
108+
store.AddOrUpdateProfile(MakeProfile("Gaming"));
109+
110+
var loaded = store.LoadProfiles();
111+
Assert.AreEqual(1, loaded.Profiles.Count);
112+
Assert.AreEqual(1, loaded.Profiles[0].Id);
113+
Assert.IsFalse(Directory.EnumerateFiles(_tempDir, "*.tmp").Any());
114+
}
115+
116+
[TestMethod]
117+
public void AddOrUpdateProfile_SaveFails_RestoresIncomingProfileState()
118+
{
119+
var profiles = new PowerDisplayProfiles { NextId = 2 };
120+
profiles.Profiles.Add(MakeProfile("Existing", id: 1));
121+
var originalJson = JsonSerializer.Serialize(profiles, ProfileSerializationContext.Default.PowerDisplayProfiles);
122+
File.WriteAllText(_profilesPath, originalJson);
123+
File.SetAttributes(_profilesPath, FileAttributes.ReadOnly);
124+
var incoming = MakeProfile("New");
125+
incoming.LastModified = DateTime.UnixEpoch;
126+
var store = CreateStore();
127+
128+
Exception? exception = null;
129+
try
130+
{
131+
store.AddOrUpdateProfile(incoming);
132+
}
133+
catch (Exception ex)
134+
{
135+
exception = ex;
136+
}
137+
138+
Assert.IsNotNull(exception);
139+
Assert.AreEqual(0, incoming.Id);
140+
Assert.AreEqual(DateTime.UnixEpoch, incoming.LastModified);
141+
Assert.AreEqual(originalJson, File.ReadAllText(_profilesPath));
142+
Assert.IsFalse(Directory.EnumerateFiles(_tempDir, "*.tmp").Any());
143+
}
144+
145+
[TestMethod]
146+
public void SaveProfiles_SaveFails_RestoresLastUpdated()
147+
{
148+
File.WriteAllText(_profilesPath, "{}");
149+
File.SetAttributes(_profilesPath, FileAttributes.ReadOnly);
150+
var profiles = new PowerDisplayProfiles { LastUpdated = DateTime.UnixEpoch };
151+
var store = CreateStore();
152+
153+
Exception? exception = null;
154+
try
155+
{
156+
store.SaveProfiles(profiles);
157+
}
158+
catch (Exception ex)
159+
{
160+
exception = ex;
161+
}
162+
163+
Assert.IsNotNull(exception);
164+
Assert.AreEqual(DateTime.UnixEpoch, profiles.LastUpdated);
165+
}
166+
167+
[TestMethod]
168+
public void AddOrUpdateProfile_TwoStoresSharingMutex_PreserveBothUpdates()
169+
{
170+
var firstStore = CreateStore();
171+
var secondStore = CreateStore();
172+
using var start = new ManualResetEventSlim();
173+
174+
var first = Task.Run(() =>
175+
{
176+
start.Wait();
177+
firstStore.AddOrUpdateProfile(MakeProfile("First"));
178+
});
179+
var second = Task.Run(() =>
180+
{
181+
start.Wait();
182+
secondStore.AddOrUpdateProfile(MakeProfile("Second"));
183+
});
184+
185+
start.Set();
186+
Task.WaitAll(first, second);
187+
188+
var loaded = firstStore.LoadProfiles();
189+
Assert.AreEqual(2, loaded.Profiles.Count);
190+
Assert.AreEqual(2, loaded.Profiles.Select(profile => profile.Id).Distinct().Count());
191+
CollectionAssert.AreEquivalent(
192+
ExpectedConcurrentProfileNames,
193+
loaded.Profiles.Select(profile => profile.Name).ToArray());
194+
}
195+
196+
[TestMethod]
197+
public void UpdateProfiles_HoldsMutexAcrossLoadModifySave()
198+
{
199+
var firstStore = CreateStore();
200+
var secondStore = CreateStore();
201+
using var updateLoaded = new ManualResetEventSlim();
202+
using var continueUpdate = new ManualResetEventSlim();
203+
204+
var first = Task.Run(() =>
205+
firstStore.UpdateProfiles(profiles =>
206+
{
207+
updateLoaded.Set();
208+
continueUpdate.Wait();
209+
profiles.SetProfile(MakeProfile("First"));
210+
return true;
211+
}));
212+
213+
Assert.IsTrue(updateLoaded.Wait(TimeSpan.FromSeconds(5)));
214+
var second = Task.Run(() => secondStore.AddOrUpdateProfile(MakeProfile("Second")));
215+
continueUpdate.Set();
216+
Task.WaitAll(first, second);
217+
218+
var loaded = firstStore.LoadProfiles();
219+
Assert.AreEqual(2, loaded.Profiles.Count);
220+
CollectionAssert.AreEquivalent(
221+
ExpectedConcurrentProfileNames,
222+
loaded.Profiles.Select(profile => profile.Name).ToArray());
223+
}
224+
225+
private ProfileStore CreateStore()
226+
{
227+
return new ProfileStore(_profilesPath, _mutexName, TimeSpan.FromSeconds(5));
228+
}
229+
230+
private static PowerDisplayProfile MakeProfile(string name, int id = 0)
231+
{
232+
return new PowerDisplayProfile(
233+
name,
234+
new List<ProfileMonitorSetting>
235+
{
236+
new ProfileMonitorSetting("MON1", 50, null, null, null),
237+
})
238+
{
239+
Id = id,
240+
};
241+
}
242+
}

src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileService.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The Microsoft Corporation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using PowerDisplay.Models;
67
using ModelsProfileHelper = PowerDisplay.Models.ProfileHelper;
78

@@ -17,7 +18,13 @@ public static class ProfileService
1718
/// <inheritdoc cref="ModelsProfileHelper.LoadProfiles"/>
1819
public static PowerDisplayProfiles LoadProfiles() => ModelsProfileHelper.LoadProfiles();
1920

21+
/// <inheritdoc cref="ModelsProfileHelper.LoadProfilesEnsuringIds"/>
22+
public static PowerDisplayProfiles LoadProfilesEnsuringIds() => ModelsProfileHelper.LoadProfilesEnsuringIds();
23+
2024
/// <inheritdoc cref="ModelsProfileHelper.SaveProfiles"/>
2125
public static bool SaveProfiles(PowerDisplayProfiles profiles) => ModelsProfileHelper.SaveProfiles(profiles);
26+
27+
/// <inheritdoc cref="ModelsProfileHelper.UpdateProfiles"/>
28+
public static bool UpdateProfiles(Func<PowerDisplayProfiles, bool> update) => ModelsProfileHelper.UpdateProfiles(update);
2229
}
2330
}

src/modules/powerdisplay/PowerDisplay.Models/PowerDisplay.Models.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
<PropertyGroup>
1818
<IsAotCompatible>true</IsAotCompatible>
1919
</PropertyGroup>
20+
<ItemGroup>
21+
<InternalsVisibleTo Include="PowerDisplay.Lib.UnitTests" />
22+
</ItemGroup>
2023
<ItemGroup>
2124
<EmbeddedResource Include="BuiltInMonitorBlacklist.json" />
2225
</ItemGroup>

0 commit comments

Comments
 (0)