Fix CRDT self-deadlock: read_llm_session re-locked the state mutex
read_llm_session acquired the CRDT_STATE mutex, then called extract_llm_session_view while holding the guard — which called our_node_id(), which locks the same non-reentrant std::sync::Mutex. The thread deadlocks itself and parks forever HOLDING the lock; every other CRDT user then queues behind it. With light traffic that's a partial wedge (MCP `show`/content reads hang while /health stays green); during a CRDT-write burst (unblock → auto-assign) enough tasks pile up to pin every tokio worker: liveness heartbeat stops, /health dies, full sled freeze. Root cause of bug 1170's repeated sled freezes, confirmed by live gdb capture: thread parked in lock_contended at presence::our_node_id ← read_llm_session ← event_matches_persona, with all other threads queued on CRDT reads. Fix: extract_llm_session_view now takes local_sled_id as a parameter; read_llm_session computes it from the guard it already holds. The trigger path (event_matches_persona on persona-subscribed WS events) explains the raciness — it needs a chat/persona event racing a pipeline transition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fHdm92yjvguPi2LiXfLB9
This commit is contained in:
@@ -61,7 +61,12 @@ pub fn read_llm_session(persona: &str) -> Option<LlmSessionView> {
|
||||
let state_mutex = get_crdt()?;
|
||||
let state = state_mutex.lock().ok()?;
|
||||
let &idx = state.llm_session_index.get(persona)?;
|
||||
extract_llm_session_view(&state.crdt.doc.llm_sessions[idx])
|
||||
// Compute the local sled id from the guard we already hold — calling
|
||||
// our_node_id() here would re-lock the same non-reentrant mutex and
|
||||
// self-deadlock the thread while it holds the lock, wedging every other
|
||||
// CRDT user behind it (bug 1170: repeated full-sled freezes).
|
||||
let local_sled_id = crate::crdt_state::hex::encode(&state.crdt.id);
|
||||
extract_llm_session_view(&state.crdt.doc.llm_sessions[idx], &local_sled_id)
|
||||
}
|
||||
|
||||
/// Atomically read new event-log entries for `persona` past the stored
|
||||
@@ -311,7 +316,14 @@ fn extract_new_event_multi(
|
||||
}
|
||||
|
||||
/// Convert a CRDT LLM session entry into its read-only view representation.
|
||||
pub(super) fn extract_llm_session_view(entry: &LlmSessionCrdt) -> Option<LlmSessionView> {
|
||||
///
|
||||
/// `local_sled_id` must be supplied by the caller: this function runs while
|
||||
/// the CRDT state lock is held, so it must NOT call `our_node_id()` (which
|
||||
/// acquires that same lock — see bug 1170).
|
||||
pub(super) fn extract_llm_session_view(
|
||||
entry: &LlmSessionCrdt,
|
||||
local_sled_id: &str,
|
||||
) -> Option<LlmSessionView> {
|
||||
let session_id = match entry.session_id.view() {
|
||||
JsonValue::String(s) if !s.is_empty() => s,
|
||||
_ => return None,
|
||||
@@ -320,8 +332,7 @@ pub(super) fn extract_llm_session_view(entry: &LlmSessionCrdt) -> Option<LlmSess
|
||||
JsonValue::String(s) => s,
|
||||
_ => String::new(),
|
||||
};
|
||||
let local_sled_id = crate::crdt_state::our_node_id().unwrap_or_default();
|
||||
let scope_filter = parse_scope(entry, &local_sled_id);
|
||||
let scope_filter = parse_scope(entry, local_sled_id);
|
||||
let high_water = parse_high_water(entry);
|
||||
Some(LlmSessionView {
|
||||
session_id,
|
||||
|
||||
Reference in New Issue
Block a user