Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions crates/tracedecay/src/daemon/branch_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1043,41 +1043,36 @@ 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<Arc<crate::mcp::McpServer>> {
pub(super) async fn mounted_project_graphs(
&self,
) -> Vec<Arc<tracedecay_project::project::TraceDecay>> {
let Ok(profile_root) = self
.profile_identity()
.and_then(|identity| authority::canonical_identity_path(identity.profile_root()))
else {
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::<Vec<_>>()
}
}

#[tracing::instrument(
name = "daemon.branch_admin.mounted_project_graphs",
level = "trace",
skip_all
)]
pub(super) async fn mounted_project_graphs(
&self,
) -> Vec<Arc<tracedecay_project::project::TraceDecay>> {
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());
Comment on lines +1074 to +1075

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

}
graphs
}
Expand Down
2 changes: 1 addition & 1 deletion crates/tracedecay/src/daemon/project_composition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
55 changes: 55 additions & 0 deletions crates/tracedecay/src/daemon/tests/ownership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions crates/tracedecay/src/mcp/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<tokio::sync::RwLock<Arc<TraceDecay>>> {
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();
Expand Down
Loading