Repository navigation
fix(daemon): list mounted graphs without leasing project servers - #4251
Conversation
A daemon-wide graph listing cloned every server Arc out of the registry and then awaited each graph read. A bounded route bind counts any extra server handle as a client lease and never evicts a leased server, so a listing parked on a graph read made an idle server unevictable and refused the open with project_server_capacity_reached. Take each server's graph slot under the registry lock, drop the lock, then read the slots. The listing holds no server handle and no registry lock across an await. Also box the release_capacity_retired_stores future that master's clippy rejects as large_futures. Refs #4175
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| for slot in &slots { | ||
| graphs.push(slot.read().await.clone()); |
There was a problem hiding this comment.
🟡 Evicted graphs remain visible to listings
If eviction removes a project while mounted_project_graphs waits on its graph slot, the listing can return that retired graph. Background refresh can then select it as a mounted graph and run against a retired project.
Learn more
A graph slot remains alive after its server leaves the registry. The listing captures slots, drops the registry lock, and waits for each slot's read lock. A concurrent capacity retirement can remove a server during that wait; its slot still yields the graph, even though the server is no longer mounted. coordinated_background_refresh_writer selects a graph from this listing and sends it into refresh work. A mere snapshot of the graph is not proof that its project remains served.
Example: Project A's slot is write-locked while the listing waits. An open for project B evicts A. When the write lock releases, the listing returns A's graph; background refresh selects A and attempts work on its retired store instead of reporting the retained graph unavailable.
Recommended fix: Tie the listing's returned graphs to registry membership at the point of use, or return a lease that keeps the selected server eligible until consumers finish. Ensure capacity eviction remains possible while the listing is parked; revalidate the selected server's identity before starting refresh or maintenance work.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Refs #4175
Cause.
StoreAdministration::mounted_project_graphscloned everyArc<McpServer>out of the registry, then awaited eachserver.cg(). A bounded route bind (bind_bounded) treats any extra server handle as a client lease and never evicts a leased server. A listing parked on a graph read (held swap) therefore made an idle server unevictable and the open was refused withproject_server_capacity_reached, which is the "eviction cleanup never parked" failure. Maintenance, doctor, branch-add, LSP and PR-autotrack all call this listing.Fix. Under the registry lock, take each server's graph slot (
McpServer::graph_slot, a clone of the innerArc<RwLock<Arc<TraceDecay>>>, not the server). Drop the lock, then read the slots. No server handle and no registry lock is held across an await.Test.
mounted_graph_listing_takes_no_server_lease_while_it_readsparks the listing on a held swap and asserts the server strong count is unchanged and the registry lock is free. On master's listing it fails (left: 5, right: 4); with the fix it passes.Also. Boxes the
release_capacity_retired_storesfuture inproject_composition.rs: master's--config=clippyalready fails onlarge_futuresthere.Verification.
:unit_test,:daemon_suite, clippy,rustfmt_check_2024green.capacity_retirement_completes_while_a_maintenance_tick_runsplus the parked-cleanup test: 30/30 at 15-way concurrency, 47/48 and 23/24 at 24-way (the failures were host stalls, see comments).