Skip to content

Fix concurrent map/struct races in sshCache and lazyManifest#6243

Merged
chrisd8088 merged 2 commits into
git-lfs:mainfrom
joshfriend:fix-ssh-cache-race
Apr 23, 2026
Merged

Fix concurrent map/struct races in sshCache and lazyManifest#6243
chrisd8088 merged 2 commits into
git-lfs:mainfrom
joshfriend:fix-ssh-cache-race

Conversation

@joshfriend

@joshfriend joshfriend commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

In #6234 sshCache.endpoints is accessed from multiple goroutines when parallel batch API workers resolve SSH credentials simultaneously. Replace the plain map with sync.Map to prevent the "concurrent map writes" panic.

lazyManifest.Upgrade() can be called concurrently, causing multiple concreteManifest instances to be created. Add a mutex to ensure only one is created.

sshCache.endpoints is accessed from multiple goroutines when parallel
batch API workers resolve SSH credentials simultaneously. Replace the
plain map with sync.Map to prevent the "concurrent map writes" panic.

lazyManifest.Upgrade() can be called concurrently, causing multiple
concreteManifest instances to be created. Add a mutex to ensure
only one is created.
@joshfriend

Copy link
Copy Markdown
Contributor Author

Side note: one of these race condition tests only reproduces when run with go test -race which is not used by CI.

@chrisd8088 chrisd8088 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey, thanks for this PR, it looks great!

IIUC, the changes in the two packages (lfshttp and tq) are unrelated to each other, so they could be separate commits. But that's merely a procedural detail, and the added thread safety is a clear improvement.

@chrisd8088

Copy link
Copy Markdown
Member

Side note: one of these race condition tests only reproduces when run with go test -race which is not used by CI.

Thanks for the note! I actually found both tests would report an issue when make test-race was run, if the corresponding code fixes were not in place, so that's good, and we could certainly add make test-race to our script/cibuild script in another PR.

@chrisd8088 chrisd8088 merged commit 4f71871 into git-lfs:main Apr 23, 2026
10 checks passed
@chrisd8088 chrisd8088 added the bug label Apr 23, 2026
@joshfriend joshfriend deleted the fix-ssh-cache-race branch April 23, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants