What's wrong
ProfileManager.RenameProfile (Keybinding/Services/ProfileManager.cs:~153-185) has two defects:
newDescription is documented as optional, but leaving it out erases the description. The new profile is built with newDescription as-is, so passing null sets Description = null instead of keeping the current value.
- It builds a brand-new
Profile and swaps it in with TryRemove followed by TryAdd. Any reference a caller already holds is now detached from the manager. That includes the objects returned by CreateProfile, CreateDefaultProfile and GetActiveProfile. Writes through that reference never reach GetProfile or SaveAsync.
Failure scenario (reproduced with a temporary MSTest)
var p = pm.CreateProfile("p", "Old", "my desc");
pm.RenameProfile("p", "New Name");
pm.GetProfile("p").Description // null (expected "my desc")
p.SetChord("x.y", chordA);
pm.GetProfile("p") has x.y? // False: the binding is silently lost
By code trace (not hit in a 20k-iteration probe): between the remove and the add, a concurrent GetProfile returns null. A concurrent CreateProfile(id) in that window makes the TryAdd fail, which drops the renamed profile and all its bindings.
Suggested fix
- Keep the existing description when
newDescription is null: newDescription ?? profile.Description.
- Rename in place. Make
Name/Description settable, internally or under the manager's lock, so the stored instance and any held references stay the same object. If replacement has to stay, use _profiles[id] = updated or TryUpdate rather than a remove/add pair.
Acceptance:
- after
RenameProfile(id, newName), the description is unchanged;
ReferenceEquals(before, GetProfile(id)) holds;
- chords set through the earlier reference are visible and persisted.
What's wrong
ProfileManager.RenameProfile(Keybinding/Services/ProfileManager.cs:~153-185) has two defects:newDescriptionis documented as optional, but leaving it out erases the description. The new profile is built withnewDescriptionas-is, so passingnullsetsDescription = nullinstead of keeping the current value.Profileand swaps it in withTryRemovefollowed byTryAdd. Any reference a caller already holds is now detached from the manager. That includes the objects returned byCreateProfile,CreateDefaultProfileandGetActiveProfile. Writes through that reference never reachGetProfileorSaveAsync.Failure scenario (reproduced with a temporary MSTest)
By code trace (not hit in a 20k-iteration probe): between the remove and the add, a concurrent
GetProfilereturns null. A concurrentCreateProfile(id)in that window makes theTryAddfail, which drops the renamed profile and all its bindings.Suggested fix
newDescriptionis null:newDescription ?? profile.Description.Name/Descriptionsettable, internally or under the manager's lock, so the stored instance and any held references stay the same object. If replacement has to stay, use_profiles[id] = updatedorTryUpdaterather than a remove/add pair.Acceptance:
RenameProfile(id, newName), the description is unchanged;ReferenceEquals(before, GetProfile(id))holds;