Skip to content

Commit 96ac65e

Browse files
committed
fix(session-roots): deadlock in record_resolution on a changed resolution
record_resolution held a DashMap read guard from `last_resolution.get()` across `last_resolution.insert()` on the same key. DashMap routes a key to a single shard RwLock, so requesting the write lock for insert() while the get() Ref is still alive self-deadlocks. The dead branch is reached exactly when a session already has a recorded resolution AND the new value differs — i.e. every time a client's effective FeatureSet *changes* (binding created/ edited, roots arrive, space reroute). That is the core path of workspace-root routing, so in production this would hang the task that records the new resolution and fires list_changed. The 64 integration tests passed because none drove a Some(..) -> different Some(..) transition; the unit test record_resolution_flips_on_change does (fallback -> bound) and hung the whole gateway lib test binary, which is how this surfaced. Fix: read the prior value into an owned bool and let the read guard drop at the end of that statement (is_some_and), then insert with no guard held. Verified: gateway lib suite now 116 passed (was hanging indefinitely). Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 9bdafa9 commit 96ac65e

1 file changed

Lines changed: 14 additions & 6 deletions

File tree

crates/mcpmux-gateway/src/services/session_roots.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,21 @@ impl SessionRootsRegistry {
151151
/// `false` when it's the same as before.
152152
pub fn record_resolution(&self, session_id: &str, fs_id: Option<&str>) -> bool {
153153
let new_val: Option<String> = fs_id.map(|s| s.to_string());
154-
match self.last_resolution.get(session_id) {
155-
Some(prev) if *prev == new_val => false,
156-
_ => {
157-
self.last_resolution.insert(session_id.to_string(), new_val);
158-
true
159-
}
154+
// IMPORTANT: read the prior value into an owned `bool` and let the
155+
// `get()` read guard drop at the end of THIS statement. Holding a
156+
// DashMap `Ref` across the `insert()` below would request a write lock
157+
// on the same shard while still holding its read lock — a self-deadlock
158+
// that fires exactly when a session's resolution changes from one
159+
// Some(..) to a different Some(..) (the common "binding changed" path).
160+
let unchanged = self
161+
.last_resolution
162+
.get(session_id)
163+
.is_some_and(|prev| *prev == new_val);
164+
if unchanged {
165+
return false;
160166
}
167+
self.last_resolution.insert(session_id.to_string(), new_val);
168+
true
161169
}
162170

163171
/// Returns every reported root across every active session, de-duplicated

0 commit comments

Comments
 (0)