Lock SetActiveProfile and DeleteProfile so a delete cannot leave a stale active id

Keybinding.Test/ProfileActivateDeleteConcurrencyTests.cs

@@ -0,0 +1,55 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Keybinding.Test;
+
+using ktsu.Keybinding.Core.Services;
+
+[TestClass]
+public class ProfileActivateDeleteConcurrencyTests
+{
+	private const int Trials = 10000;
+
+	public TestContext TestContext { get; set; } = null!;
+
+	[TestMethod]
+	public void SetActiveProfile_RacingDeleteProfile_NeverLeavesTheDeletedIdActive()
+	{
+		int stale = 0;
+		for (int trial = 0; trial < Trials; trial++)
+		{
+			ProfileManager profiles = new();
+			profiles.CreateProfile("x", "X");
+			using Barrier barrier = new(2);
+
+			Thread activate = new(() =>
+			{
+				barrier.SignalAndWait(TestContext.CancellationToken);
+
+				// Keep activating until the delete lands, so the activation is in flight when it does
+				while (profiles.SetActiveProfile("x"))
+				{
+					// Intentionally empty: each iteration is another activation racing the delete
+				}
+			});
+			Thread delete = new(() =>
+			{
+				barrier.SignalAndWait(TestContext.CancellationToken);
+				profiles.DeleteProfile("x");
+			});
+
+			activate.Start();
+			delete.Start();
+			activate.Join();
+			delete.Join();
+
+			// A profile recreated with the deleted id must not become active on its own
+			profiles.CreateProfile("x", "Recreated");
+			if (profiles.GetActiveProfile() is not null)
+			{
+				stale++;
+			}
+		}
+
+		Assert.AreEqual(0, stale, $"{stale} of {Trials} trials left the active id pointing at the deleted profile.");
+	}
+}

Keybinding/Services/ProfileManager.cs

@@ -32,7 +32,7 @@
 	{
 		if (string.IsNullOrWhiteSpace(id))
 		{
 			throw new ArgumentException("Profile ID cannot be null or whitespace", nameof(id));
 		}
 
 		if (string.IsNullOrWhiteSpace(name))
@@ -57,15 +57,22 @@
 		}
 
 		string normalizedId = profileId.Trim();
-		bool removed = _profiles.TryRemove(normalizedId, out _);
 
-		// Clear active profile if it was the one being deleted
-		if (removed && _activeProfileId == normalizedId)
+		// Remove and clear under the lock SetActiveProfile checks and sets under, so an activation
+		// cannot land between them and leave the active id naming the deleted profile
+		// (ktsu-dev/Keybinding#120)
+		lock (_lock)
 		{
-			_activeProfileId = null;
-		}
+			bool removed = _profiles.TryRemove(normalizedId, out _);
 
-		return removed;
+			// Clear active profile if it was the one being deleted
+			if (removed && _activeProfileId == normalizedId)
+			{
+				_activeProfileId = null;
+			}
+
+			return removed;
+		}
 	}
 
 	/// <inheritdoc/>
@@ -73,7 +80,7 @@
 	{
 		return string.IsNullOrWhiteSpace(profileId)
 			? throw new ArgumentException("Profile ID cannot be null or whitespace", nameof(profileId))
 			: _profiles.TryGetValue(profileId.Trim(), out Profile? profile) ? profile : null;
 	}
 
 	/// <inheritdoc/>
@@ -102,13 +109,16 @@
 
 		string normalizedId = profileId.Trim();
 
-		if (!_profiles.ContainsKey(normalizedId))
+		lock (_lock)
 		{
-			return false;
-		}
+			if (!_profiles.ContainsKey(normalizedId))
+			{
+				return false;
+			}
 
-		_activeProfileId = normalizedId;
-		return true;
+			_activeProfileId = normalizedId;
+			return true;
+		}
 	}
 
 	/// <inheritdoc/>