You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
A lock created or released during an in-flight lock-list refresh is missing from listings for a full ListTtl: the refresh publishes its stale walk after the invalidation ran #70
Publish doesn't check for invalidation.LockListService.RefreshAsync publishes whatever the walk returned, unconditionally: snapshots.Publish(key, result.Snapshot!) at GitLfsCache/Locks/LockListService.cs:155. The fan-out's resolution refresh does the same (Locks/LockFanOut.cs:273).
Invalidation leaves no trace.LockSnapshotStore.Invalidate (Locks/LockSnapshotStore.cs:36-48) removes only the snapshots that exist when it runs. Publish (:26-32) overwrites without looking, so a walk that was already running when the invalidation happened still gets published.
The stale result looks fresh. The snapshot is timestamped when the walk finishes (Locks/LockListRefresher.cs:127, new LockSnapshot(collected, timeProvider.GetUtcNow())), so it is treated as fresh for the whole ListTtl.
Failure scenario
The Unreal plugin lists locks every 30 s from every editor, so on a busy repository a multi-page walk is often in progress.
Client A's GET locks becomes the refresh leader and starts walking pages. With thousands of locks this takes seconds.
Client B locks a file. The POST locks is relayed, upstream answers 201, and LockRouteHandler.InvalidateIfChanged (Endpoints/LockRouteHandler.cs:230-235) calls Invalidate. There's nothing to remove, because the previous snapshot has already expired or been dropped.
A's walk finishes. Its pages were read before B's lock existed, but it publishes them stamped with the current time.
For the next ListTtl (15 s by default), every admitted client, B included, is served a listing without B's lock. An unlock fails the same way in reverse: the file still shows as locked.
A locks/batch fan-out that invalidates while a listing walk is running hits the same gap (LockFanOut.cs:89).
This breaks the guarantee the invalidation exists for ("the change this client just made is visible to the next listing", LockRouteHandler.cs:164-166). #46/#60 made invalidation cover every ref, but didn't close this race. No test exercises an invalidation during a refresh.
Suggested fix
Keep a per-repository generation counter in LockSnapshotStore, and have Invalidate increment it.
Capture the generation before the walk starts, and publish only if it hasn't changed. For example, Publish(key, snapshot, expectedGeneration) returns false otherwise, and the caller serves the result to its own requester without caching it.
Optionally, also stamp the snapshot with the time the walk started.
Acceptance criteria
A test with a refresher that blocks mid-walk: call Invalidate for that repository while it's blocked, then release the refresher. The next ResolveAsync must refresh again rather than serve the pre-invalidation walk.
The same guard applies to LockFanOut.RefreshForResolutionAsync.
Priority: High. This is a race, but it is common on a busy repository because the Unreal plugin polls every 30 s. For up to ListTtl, clients are shown a lock set that is wrong: a lock someone just took is missing, or a released lock still shows. That is the guarantee invalidation exists to provide, and it can lead to conflicting edits of locked binary assets.
Area / suggested assignee: Locks subsystem (Locks/LockSnapshotStore.cs, LockListService.cs, LockFanOut.cs), @matt-edmondson.
Activity
matt-edmondson commentedon Sep 28, 2026
Triage
ListTtl, clients are shown a lock set that is wrong: a lock someone just took is missing, or a released lock still shows. That is the guarantee invalidation exists to provide, and it can lead to conflicting edits of locked binary assets.Locks/LockSnapshotStore.cs,LockListService.cs,LockFanOut.cs), @matt-edmondson.Publishis a small, testable change. Land it before or together with Unlocking many paths through locks/batch can run a full lock-list walk per path, outside single-flight and the upstream limiter #63 and Unlock by path fails with 404/409 when the snapshot names a released-and-reissued lock id: the spec's one refresh + one retry isn't implemented #71, which touch the same resolution-refresh path.Generated by Claude Code