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
A duplication review of #5934 (server→client routing in the streamable proxy) found the repo now has three near-identical hand-rolled "per-key mutex" implementations of the same two-phase pattern (acquire a global mutex → look up/create a per-key sync.Mutex → release the global mutex → block on the per-key mutex):
pkg/transport/proxy/streamable/keyed_mutex.go — keyedMutex (added in Route server-to-client MCP messages per session #5934; ref-counted / self-evicting because keys are arbitrary client-supplied resource URIs with unbounded cardinality, and it is deliberately held across a blocking upstream round-trip)
The keyedMutex variant intentionally diverges (eviction is required to avoid an unbounded-map leak — do not collapse it into the never-evicting variants), but the core acquire/release logic is duplicated three ways.
Proposed work
Extract a single generic keyed-mutex into an internal package (e.g. pkg/keyedmutex) supporting both modes:
non-evicting (for the small, bounded pluginsvc/skillsvc key sets), and
evicting / ref-counted (for unbounded, client-supplied keys like the streamable proxy's URIs).
Port all three call sites to it. Preserve each site's current semantics exactly (especially the streamable proxy's ref-counted eviction and its "held across a blocking call" usage — see the doc comment on keyedMutex).
Notes
Not a behavior change; pure consolidation + tests for the shared package.
Context
A duplication review of #5934 (server→client routing in the streamable proxy) found the repo now has three near-identical hand-rolled "per-key mutex" implementations of the same two-phase pattern (acquire a global mutex → look up/create a per-key
sync.Mutex→ release the global mutex → block on the per-key mutex):pkg/plugins/pluginsvc/service.go—pluginLock(never evicts; key cardinality bounded by installed plugins)pkg/skills/skillsvc/service.go—skillLock(never evicts; bounded)pkg/transport/proxy/streamable/keyed_mutex.go—keyedMutex(added in Route server-to-client MCP messages per session #5934; ref-counted / self-evicting because keys are arbitrary client-supplied resource URIs with unbounded cardinality, and it is deliberately held across a blocking upstream round-trip)The
keyedMutexvariant intentionally diverges (eviction is required to avoid an unbounded-map leak — do not collapse it into the never-evicting variants), but the core acquire/release logic is duplicated three ways.Proposed work
Extract a single generic keyed-mutex into an internal package (e.g.
pkg/keyedmutex) supporting both modes:pluginsvc/skillsvckey sets), andPort all three call sites to it. Preserve each site's current semantics exactly (especially the streamable proxy's ref-counted eviction and its "held across a blocking call" usage — see the doc comment on
keyedMutex).Notes
Generated with Claude Code