Skip to content

Fix concurrent map panic in MCP server caches#16042

Merged
pelikhan merged 3 commits intogithub:mainfrom
AI-Reviewer-QS:fix/mcp-server-concurrent-map-panic
Feb 16, 2026
Merged

Fix concurrent map panic in MCP server caches#16042
pelikhan merged 3 commits intogithub:mainfrom
AI-Reviewer-QS:fix/mcp-server-concurrent-map-panic

Conversation

@AI-Reviewer-QS
Copy link
Contributor

Summary

The MCP server's permissionCache (map) and repoCache (pointer) are package-level variables accessed concurrently from HTTP handler goroutines without any synchronization. In Go, concurrent read+write on a plain map causes a fatal runtime panic (concurrent map read and map write), crashing the server.

This is triggered when the MCP server runs in HTTP transport mode (--port) and receives multiple simultaneous requests that hit queryActorRole or getRepository.

Fix: Add a sync.RWMutex (cacheMu) to protect both caches. Read operations use RLock for concurrency, write operations use Lock for exclusive access.

Affected code paths:

  • getRepository() — reads/writes repoCache pointer (lines 65-101)
  • queryActorRole() — reads/writes/deletes from permissionCache map (lines 118-150)

Test plan

  • Added TestQueryActorRole_ConcurrentCacheAccess — 20 goroutines × 100 iterations exercising concurrent cache read/write/delete
  • Added TestGetRepository_ConcurrentCacheAccess — same pattern for the repo cache
  • Both tests pass with go test -race (no data races detected)
  • Verified the race is detected without the fix via go test -race

The permissionCache map and repoCache pointer are accessed concurrently
from HTTP handler goroutines without synchronization. In Go, concurrent
read+write on a map causes a fatal runtime panic ("concurrent map read
and map write").

Add a sync.RWMutex to protect both caches. Read operations use RLock
for concurrency, write operations use Lock for exclusive access.
Copy link
Contributor

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

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

Create a class to hide the details and apply locks consistentely.

Replace bare global mutex + maps with an mcpCacheStore struct that
encapsulates all locking internally. Callers now use GetPermission,
SetPermission, GetRepo, and SetRepo methods instead of manual
lock/unlock sequences, making it impossible to access caches without
proper synchronization.
repoCache *repositoryCache
repoCacheTTL = 1 * time.Hour
)
func newMCPCacheStore() *mcpCacheStore {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move cache to own file

@pelikhan pelikhan merged commit 2884c51 into github:main Feb 16, 2026
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants