From ee96ca8eea92de717b954f28c026ee7a6fe45dfe Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sun, 18 Oct 2026 05:24:45 +0000 Subject: [PATCH] fix(daemon): list mounted graphs without leasing project servers 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 --- crates/tracedecay/src/daemon/branch_admin.rs | 33 +++++------ .../src/daemon/project_composition.rs | 2 +- .../tracedecay/src/daemon/tests/ownership.rs | 55 +++++++++++++++++++ crates/tracedecay/src/mcp/server.rs | 7 +++ 4 files changed, 77 insertions(+), 20 deletions(-) diff --git a/crates/tracedecay/src/daemon/branch_admin.rs b/crates/tracedecay/src/daemon/branch_admin.rs index c62714968c..c0d59b67ce 100644 --- a/crates/tracedecay/src/daemon/branch_admin.rs +++ b/crates/tracedecay/src/daemon/branch_admin.rs @@ -1043,11 +1043,13 @@ impl StoreAdministration { } #[tracing::instrument( - name = "daemon.branch_admin.mounted_project_servers", + name = "daemon.branch_admin.mounted_project_graphs", level = "trace", skip_all )] - pub(super) async fn mounted_project_servers(&self) -> Vec> { + pub(super) async fn mounted_project_graphs( + &self, + ) -> Vec> { let Ok(profile_root) = self .profile_identity() .and_then(|identity| authority::canonical_identity_path(identity.profile_root())) @@ -1055,29 +1057,22 @@ impl StoreAdministration { return Vec::new(); }; - { + // A cloned server handle counts as a client lease, and a bounded route + // bind never evicts a leased server. Take the graph slots under the + // registry lock and read them after it drops, so a held graph swap + // parks neither a route bind nor an eviction. + let slots = { let servers = self.project_servers.lock().await; servers .servers .iter() .filter(|(key, _)| key.owner.profile_root == profile_root) - .map(|(_, entry)| Arc::clone(&entry.server)) + .map(|(_, entry)| entry.server.graph_slot()) .collect::>() - } - } - - #[tracing::instrument( - name = "daemon.branch_admin.mounted_project_graphs", - level = "trace", - skip_all - )] - pub(super) async fn mounted_project_graphs( - &self, - ) -> Vec> { - let servers = self.mounted_project_servers().await; - let mut graphs = Vec::with_capacity(servers.len()); - for server in &servers { - graphs.push(server.cg().await); + }; + let mut graphs = Vec::with_capacity(slots.len()); + for slot in &slots { + graphs.push(slot.read().await.clone()); } graphs } diff --git a/crates/tracedecay/src/daemon/project_composition.rs b/crates/tracedecay/src/daemon/project_composition.rs index abfdccae92..5b28d35b8b 100644 --- a/crates/tracedecay/src/daemon/project_composition.rs +++ b/crates/tracedecay/src/daemon/project_composition.rs @@ -212,7 +212,7 @@ async fn release_capacity_unless_re_served( .keys() .any(|key| key.owner.project_id == stores.owner.project_id); if !re_served { - release_capacity_retired_stores(stores).await?; + Box::pin(release_capacity_retired_stores(stores)).await?; } drop(capacity_admission); Ok(()) diff --git a/crates/tracedecay/src/daemon/tests/ownership.rs b/crates/tracedecay/src/daemon/tests/ownership.rs index 1b93654293..720a04b26a 100644 --- a/crates/tracedecay/src/daemon/tests/ownership.rs +++ b/crates/tracedecay/src/daemon/tests/ownership.rs @@ -339,6 +339,61 @@ async fn project_server_cache_hit_skips_open_and_singleflights_first_miss() { tracedecay_runtime_core::git_repository::reset_repository_discovery_for_test(&project); } +/// A daemon-wide graph listing must not count as a client of the servers it +/// lists. A bounded route bind treats any extra server handle as a lease, so +/// a listing that held handles across its graph reads made an idle server +/// unevictable and refused the open with `project_server_capacity_reached`. +#[cfg(unix)] +#[tokio::test] +async fn mounted_graph_listing_takes_no_server_lease_while_it_reads() { + let temp = TempDir::new().expect("temp dir"); + let project = temp.path().join("project"); + let profile_root = temp.path().join("profile"); + std::fs::create_dir_all(project.join("src")).expect("project dir"); + std::fs::write(project.join("src/main.rs"), "fn main() {}\n").expect("project source"); + let client_identity = test_client_identity_for(profile_root.clone()); + initialize_test_project(&project, &client_identity).await; + let handshake = DaemonHandshake { + project_path: Some(project.clone()), + client_identity, + ..test_handshake_defaults() + }; + let _database_scope = enter_test_daemon_database_scope(&profile_root, "graph-listing-lease"); + let engine = test_daemon_engine_for_profile(&profile_root); + let server = engine + .project_server(&handshake) + .await + .expect("mount project server"); + + let swap = server.graph_slot().write_owned().await; + let listing = engine.store_administration.mounted_project_graphs(); + let mut listing = std::pin::pin!(listing); + let handles = Arc::strong_count(&server); + assert!( + std::future::Future::poll( + listing.as_mut(), + &mut std::task::Context::from_waker(std::task::Waker::noop()), + ) + .is_pending(), + "the listing parks on the held graph read" + ); + assert_eq!( + Arc::strong_count(&server), + handles, + "a parked graph listing must hold no server handle" + ); + assert!( + engine + .store_administration + .project_servers() + .try_lock() + .is_ok(), + "a parked graph listing must not hold the registry lock, or every route bind waits on the swap" + ); + drop(swap); + assert_eq!(listing.await.len(), 1); +} + /// A mounted route alias answers without re-running registry admission: the /// retirement/revocation lifecycle owns a mounted server's validity, and only /// a (re)mount re-enters `ensure_registered_project_route`. A tombstone diff --git a/crates/tracedecay/src/mcp/server.rs b/crates/tracedecay/src/mcp/server.rs index 6a1afd5127..0fe82fc0f2 100644 --- a/crates/tracedecay/src/mcp/server.rs +++ b/crates/tracedecay/src/mcp/server.rs @@ -1328,6 +1328,13 @@ impl McpServer { self.cg.read().await.clone() } + /// The served-instance slot, for readers that must not keep the server + /// itself alive: a server handle counts as a client lease and blocks + /// bounded eviction. + pub(crate) fn graph_slot(&self) -> Arc>> { + Arc::clone(&self.cg) + } + #[tracing::instrument(name = "mcp.server.stats_snapshot", level = "trace", skip_all)] pub async fn server_stats_json(&self) -> Value { let uptime = self.stats.started_at.elapsed();