Skip to content

Event feed: FileStore's per-path lock is keyed on casing, so one file can take two mutexes #761

Description

@jeremy

Raised by Codex on #705 (filestore.go, the per-path mutex registry).

FileStore keys its mutex registry on filepath.Abs(path), and Abs
preserves component casing. On a case-insensitive filesystem — macOS's default
APFS, and Windows — two stores constructed with paths differing only in casing
address the same checkpoint file but take different mutexes. Concurrent
Save calls then both read the old JSON, and both rename their independent
updates into place: one lineage is silently dropped, against the documented
per-path concurrency guarantee.

Naming the actor, because it decides whether this is worth fixing: there is no
adversary. It is a plain mistake — one path from an env var, another
hardcoded; a ~/Library vs ~/library; a config file edited by hand. The
consequence is a silently lost checkpoint lineage, which surfaces later as a
feed re-entering at a position it should have moved past.

Why it wants a decision

Three shapes, and they are not equivalent:

  • Lowercase the key on case-insensitive platforms. A runtime.GOOS check
    (darwin, windows). Cheap, but wrong at the edges: macOS can be formatted
    case-sensitive, and Linux can mount case-insensitive filesystems, so the
    heuristic both over- and under-normalizes. Over-normalizing is the dangerous
    direction — it would serialize two genuinely distinct files on a
    case-sensitive macOS volume, which is merely slow, so the failure is benign.
  • Derive the key from OS-level file identity (device + inode via
    os.SameFile/Stat). Correct by construction, but the file need not exist
    when the store is constructed, so it needs a create-or-stat at registration
    and a fallback for the not-yet-created case.
  • Narrow the documented guarantee. Say the per-path lock is per exact
    path and that two stores addressing one file by different spellings are the
    caller's problem. Free, and honest, but it moves a footgun into the docs
    where nothing enforces it.

I lean toward the first for the reason its own edge case is benign, but it is a
published concurrency guarantee on a public type, so it wants a call rather
than a quiet heuristic.

A regression test wants care too: asserting the lock identity (a unit test on
the key derivation) is deterministic, whereas asserting the lost update is a
race and would be flaky.

Follows #606, #614, #645, #696; siblings #753, #758, #759.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggo

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions