Skip to content

Fix/cross process index locking#853

Open
Screendead wants to merge 3 commits intoforcedotcom:mainfrom
Screendead:fix/cross-process-index-locking
Open

Fix/cross process index locking#853
Screendead wants to merge 3 commits intoforcedotcom:mainfrom
Screendead:fix/cross-process-index-locking

Conversation

@Screendead
Copy link
Copy Markdown

Summary

Adds cross-process file locking around all ShadowRepo methods that read or write the isomorphic-git index file, preventing the intermittent Invalid checksum in GitIndex buffer errors that occur when SF CLI and the VS Code Salesforce Extension operate concurrently.

The lock uses lockInit from @salesforce/core (which wraps proper-lockfile), already a transitive dependency. No new dependencies are introduced.

Problem

isomorphic-git's GitIndexManager uses an in-memory AsyncLock that only serializes within a single Node.js process. The actual filesystem lock is commented out. When two separate processes (SF CLI in the terminal and the VS Code extension running in the background) both access .sf/orgs/<orgId>/localSourceTracking/index, the index file gets corrupted.

This has been reported across both repos for 3+ years: forcedotcom/cli#3328, forcedotcom/cli#2422, forcedotcom/cli#2483, isomorphic-git/isomorphic-git#1828, isomorphic-git/isomorphic-git#1721. The isomorphic-git maintainers have stated cross-process coordination is the integrator's responsibility.

Changes

Single file changed: src/shared/local/localShadowRepo.ts

  • Import lockInit from @salesforce/core
  • Add withGitLock() private method that acquires a cross-process file lock on <gitDir>/index via proper-lockfile's atomic mkdir-based mechanism
  • Wrap getStatus(), commitChanges(), gitInit(), and delete() with withGitLock()
  • withGitLock() is reentrant within the same process (tracked via a boolean flag) to handle the internal call chain getStatus()detectMovedFiles()commitChanges()getStatus(true) without deadlocking

Reads are locked too; a concurrent write can leave a partially-written index that statusMatrix would misparse.

Caveats

  • proper-lockfile's lock only works if all processes check for it. SF CLI and the VS Code extension bundle their own copies of @salesforce/source-tracking. Full protection is only in effect once both consumers ship this update. During the transition (one patched, one not), the collision window is reduced but not eliminated.
  • The reproduction script I've used to validate the fix produces more severe corruption (hung parser from garbled entry data) than the typical user-facing error (checksum mismatch with path fragments in the trailing bytes). Both are caused by the same root issue (no file locking) but the specific byte-level corruption pattern depends on exact timing.

Test plan

  • yarn compile - clean
  • yarn test - all 105 tests pass (98 existing + 7 new)
  • yarn lint - clean
  • npx knip - no new unused exports or dependencies
  • New test: lock is held during commitChanges() (verified via index.lock directory presence)
  • New test: lock is held during getStatus()
  • New test: lock skipped for empty commitChanges() (no files)
  • New test: lock skipped for cached getStatus() (no filesystem access)
  • New test: reentrant getStatus()detectMovedFiles()commitChanges() completes without deadlock
  • New test: lock released after operation throws
  • New test: delete() succeeds despite removing the lock directory
  • Manual: two concurrent sf project deploy preview commands serialize correctly (second process waits for first to release lock, both complete cleanly)

Screendead and others added 2 commits April 21, 2026 15:08
…rations

isomorphic-git's GitIndexManager uses an in-memory AsyncLock that only
serializes within a single Node.js process. When SF CLI and the VS Code
extension both access the same source tracking index as separate
processes, the index file gets corrupted.

Add a reentrant withGitLock() wrapper using lockInit from @salesforce/core
(proper-lockfile) around getStatus(), commitChanges(), gitInit(), and
delete(). No new dependencies.

Closes forcedotcom/cli#3328
…xManager.acquire

Co-authored-by: Luke Cotter <lcotter@certinia.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds cross-process locking around ShadowRepo operations that touch the isomorphic-git index to prevent concurrent access corruption when multiple Node processes (e.g., SF CLI + VS Code extension) operate on the same shadow repo.

Changes:

  • Introduces a withGitLock() helper using @salesforce/core lockInit and wraps gitInit(), getStatus(), commitChanges(), and delete().
  • Refactors commitChanges() to write blobs and apply index updates in a single GitIndexManager.acquire() call before committing.
  • Updates/adds unit tests to validate lock acquisition/release, caching behavior, reentrancy, and error cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/shared/local/localShadowRepo.ts Adds cross-process index locking and refactors index updates in commitChanges()
test/unit/localShadowRepo.test.ts Adds unit tests for lock behavior and updates blob-write expectations
test/unit/localDetectMovedFiles.test.ts Updates moved-file tests to assert commit behavior consistent with the new implementation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/shared/local/localShadowRepo.ts
Comment thread src/shared/local/localShadowRepo.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants