fix(tools): guard Registry.tools with RWMutex against concurrent MCP registration#709
Merged
Merged
Conversation
…registration The tool registry's tools map was written by the MCP liveness-probe goroutine (RegisterMCPServerTools/UnregisterMCPServerTools) while the main loop read it (GetTool/ListAvailableTools/GetToolDefinitions/ IsToolEnabled) with no synchronization - a data race flagged by -race and a potential concurrent map read/write panic. Guard the map with a sync.RWMutex: RLock in the read accessors, Lock in all post-construction writers (MCP register/unregister, SetMemoryBackend, SetScreenshotProvider, SetScreenshotServer). registerTools stays lock-free since it runs before the registry escapes the constructor. Add a -race regression test that churns MCP tool registration from a goroutine while the read accessors run concurrently. Closes #708
inference-gateway-releaser Bot
added a commit
that referenced
this pull request
Jul 2, 2026
## [0.127.0](v0.126.0...v0.127.0) (2026-07-02) ### 🚀 Features * **plan:** compact context and start a fresh session when a plan is approved ([#705](#705)) ([28ddba7](28ddba7)) * **memory:** configurable git sync backend ([#707](#707)) ([6a68c3d](6a68c3d)), closes [#670](#670) [#670](#670) * **history:** each subagent gets its own history file ([#711](#711)) ([16c8fa1](16c8fa1)), closes [#699](#699) ### 🐛 Bug Fixes * **plan:** accepting a plan switches to auto-accept mode by default ([#714](#714)) ([7b0befc](7b0befc)), closes [#710](#710) * **tools:** guard concurrent gitignore and screenshot caches ([#713](#713)) ([0ff4276](0ff4276)), closes [#712](#712) * **tools:** guard Registry.tools with RWMutex against concurrent MCP registration ([#709](#709)) ([8a70871](8a70871)), closes [#708](#708) * **ui:** update dim color from [#565f89](https://github.com/inference-gateway/cli/issues/565f89) to [#7a7f9a](https://github.com/inference-gateway/cli/issues/7a7f9a) for improved contrast ([#706](#706)) ([b2ee67e](b2ee67e)) ### 🔧 Build System * **deps:** bump github.com/go-telegram/bot from 1.21.0 to 1.22.0 in the gomod group ([#703](#703)) ([3de5136](3de5136)) * **deps:** bump the github-actions group with 5 updates ([#704](#704)) ([80221f7](80221f7))
Contributor
|
🎉 This PR is included in version 0.127.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the data race on the tool
Registry.toolsmap reported in #708: the MCP liveness-probe goroutine registers/unregistersMCP_*tools (RegisterMCPServerTools/UnregisterMCPServerTools) while the main loop reads the same map (GetTool/ListAvailableTools/GetToolDefinitions/IsToolEnabled) with no synchronization — flagged by-race, and a potentialfatal error: concurrent map read and map writepanic.Changes
internal/agent/tools/registry.go: addtoolsMu sync.RWMutexguardingtools. The four read accessors takeRLock; every post-construction writer takesLock— the MCP probe paths plusSetMemoryBackend,SetScreenshotProvider, andSetScreenshotServer.registerToolsdeliberately stays lock-free (documented): it runs insideNewRegistrybefore the registry is shared, and no tool constructor calls back into a locking accessor, so there is no re-entrancy hazard.internal/agent/tools/registry_test.go:TestRegistry_ConcurrentMCPToolAccessregression test — churns MCP register/unregister from a goroutine (minimalstubMCPManager+ counterfeiterFakeMCPClient) while the test goroutine hammers all four read accessors.Verification
-racewith multiple DATA RACE reports — it locks in the regression.go test -race ./internal/agent/tools/passes,golangci-lint run ./internal/agent/tools/...reports 0 issues,go build ./...compiles clean.Closes #708