stacklok / stacklok/toolhive

Unify the three keyed-mutex implementations into a shared package

Open
#5,937 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

chore
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

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.gopluginLock (never evicts; key cardinality bounded by installed plugins)
  • pkg/skills/skillsvc/service.goskillLock (never evicts; bounded)
  • pkg/transport/proxy/streamable/keyed_mutex.gokeyedMutex (added in #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.
  • Confirmed as a follow-up (not a blocker) by the #5934 review panel.

Generated with Claude Code

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the three existing implementations in pkg/plugins/pluginsvc/service.go, pkg/skills/skillsvc/service.go, and pkg/transport/proxy/streamable/keyed_mutex.go, including the streamable proxy's doc comment. Then inspect their current usage and tests, and define the shared package's non-evicting and ref-counted modes. Done means all three call sites use it while preserving their existing locking, eviction, and blocking-call semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.