Fix #214: Prevent ToolManager crash callback deadlock
- Move crash_callback execution outside of self._lock - Callback is called after releasing the monitor lock - Re-acquire lock only for restart decision logic - Prevents deadlock if callback tries to acquire same lock - Maintains thread safety for restart and process state updates
This commit is contained in:
+22
-17
@@ -336,28 +336,33 @@ class ToolManager:
|
||||
"restart_count": tool.restart_count},
|
||||
source_module="tool_manager")
|
||||
|
||||
# Crash callback (e.g., corrective ARP)
|
||||
if tool.crash_callback:
|
||||
# Call crash callback OUTSIDE lock to prevent deadlock
|
||||
callback = tool.crash_callback
|
||||
restart_needed = tool.restart_count < tool.max_restarts
|
||||
|
||||
# Release lock before callback
|
||||
if callback:
|
||||
try:
|
||||
tool.crash_callback()
|
||||
callback()
|
||||
except Exception:
|
||||
logger.exception("Crash callback failed for %s", tool.name)
|
||||
|
||||
# Auto-restart with exponential backoff
|
||||
if tool.restart_count < tool.max_restarts:
|
||||
backoff = min(2 ** tool.restart_count, 30)
|
||||
logger.info("Restarting %s in %ds (attempt %d/%d)",
|
||||
tool.name, backoff, tool.restart_count + 1,
|
||||
tool.max_restarts)
|
||||
time.sleep(backoff)
|
||||
tool.restart_count += 1
|
||||
with self._lock:
|
||||
# Re-acquire lock for restart decision
|
||||
with self._lock:
|
||||
# Auto-restart with exponential backoff
|
||||
if tool.restart_count < tool.max_restarts:
|
||||
backoff = min(2 ** tool.restart_count, 30)
|
||||
logger.info("Restarting %s in %ds (attempt %d/%d)",
|
||||
tool.name, backoff, tool.restart_count + 1,
|
||||
tool.max_restarts)
|
||||
time.sleep(backoff)
|
||||
tool.restart_count += 1
|
||||
self._start_tool(tool)
|
||||
else:
|
||||
logger.error("Tool %s exceeded max restarts (%d), giving up",
|
||||
tool.name, tool.max_restarts)
|
||||
tool.process = None
|
||||
tool.pid = None
|
||||
else:
|
||||
logger.error("Tool %s exceeded max restarts (%d), giving up",
|
||||
tool.name, tool.max_restarts)
|
||||
tool.process = None
|
||||
tool.pid = None
|
||||
|
||||
elif tool.health_check:
|
||||
# Process alive — run health check
|
||||
|
||||
Reference in New Issue
Block a user