镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Two app instances saving at once silently lose each other's profiles and commands: the data-directory lock only serializes within one process #236

Open

Description

@matt-edmondson

What's wrong

Every read-modify-write in JsonKeybindingRepository is serialized by a static ConcurrentDictionary<string, SemaphoreSlim> keyed by data directory:

// One lock per data directory, shared by every repository in the process over that directory
private static readonly ConcurrentDictionary<string, SemaphoreSlim> DirectoryLocks = new(StringComparer.Ordinal);

That lock only works inside one process. SaveProfileAsync and UpdateCommandsAsync each load the file, merge in memory and write the result back. When two processes do this over the same directory at the same time, the last writer replaces the other's changes. The class remarks and the README scope the guarantee to "in the process", so this is a gap left by the #148/#195 fixes rather than a regression. But two instances of a desktop app sharing one keybinding directory is a common setup, for example two editor windows. In that setup a user's new profile or rebind can disappear with no error.

Repro

A console app referencing Keybinding.csproj:

// mode "proc <id> <dir>"
using var m = new KeybindingManager(dir);
await m.InitializeAsync();
m.Commands.RegisterCommand(new Command("cmd." + id, "Cmd " + id));
m.Profiles.CreateProfile("p" + id, "P " + id);
await m.SaveAsync();
// mode "check <dir>": fresh manager, InitializeAsync, print profile/command counts
for i in $(seq 1 8); do app proc $i $D & done; wait; app check $D
  • Expected: profiles=8 commands=8
  • Actual, 8 processes, 3 runs: profiles=3 commands=2, profiles=6 commands=4, profiles=3 commands=1
  • Actual, 4 processes, 5 runs: 2/1, 1/1, 1/1, 2/2, 2/1

Every run lost data.

Suggested fix / acceptance criteria

  • Hold an OS-level lock for the whole load-merge-write, alongside the in-process semaphore. For example, open <dir>/.lock with FileShare.None and retry with backoff on IOException.
  • Cover SaveProfileAsync, UpdateCommandsAsync, DeleteProfileAsync and SaveActiveProfileAsync.
  • Add a test that runs N concurrent savers which can't share the static lock (separate processes, or separate AssemblyLoadContexts), and assert that all N profiles and commands survive.
  • Update the class remarks and the README to say saves are serialized across processes.

Activity

  1. matt-edmondson commented on Nov 8, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions