Thread/async safety: unprotected global mutable state causes race conditions in multi-agent deployments

[MervinPraison]

Summary

Multiple critical concurrency bugs violate the "multi-agent + async safe by default" principle. These can cause RuntimeError, lost data, and crashes in production multi-agent deployments.

Specific Issues

1. Unprotected global dicts in agents/agents.py (lines 33-35)

# NO lock protection — contrast with agent.py which HAS _server_lock
_agents_server_started = {}
_agents_registered_endpoints = {}
_agents_shared_apps = {}

Multiple agents starting API servers concurrently can race on these dicts. agent/agent.py correctly uses _server_lock = threading.Lock() for the same pattern — agents.py does not.

2. Race condition in global ToolRegistry singleton (tools/registry.py, lines 256-261)

_global_registry: Optional[ToolRegistry] = None

def get_registry() -> ToolRegistry:
    global _global_registry
    if _global_registry is None:        # Thread A reads None
        _global_registry = ToolRegistry()  # Thread B also reads None → two registries created
    return _global_registry

No lock around initialization. Two threads can create separate registries, causing tools registered in one to be invisible in the other.

Fix: Add double-checked locking:

_registry_lock = threading.Lock()

def get_registry() -> ToolRegistry:
    global _global_registry
    if _global_registry is None:
        with _registry_lock:
            if _global_registry is None:
                _global_registry = ToolRegistry()
    return _global_registry

3. asyncio.run() inside potentially-async context (agent/agent.py, line 5067)

if hasattr(backend, 'request_approval_sync'):
    decision = backend.request_approval_sync(request)
else:
    decision = asyncio.run(backend.request_approval(request))  # 💥 RuntimeError if event loop running

When _check_tool_approval_sync() is called during achat() or async execution, asyncio.run() will raise RuntimeError: asyncio.run() cannot be called from a running event loop. The code should detect whether an event loop is running and use asyncio.get_running_loop().create_task() or similar instead.

4. Unprotected _pending_approvals dict (agent/agent.py, lines 1630, 8334-8384)

self._pending_approvals = {}  # No lock

# Write (async method):
self._pending_approvals[tracking_id] = {...}

# Read+Delete (concurrent method):
for tid, info in self._pending_approvals.items():  # RuntimeError: dict changed size
    del self._pending_approvals[tid]

Concurrent async tasks modifying this dict can cause RuntimeError: dictionary changed size during iteration.

Impact

• Production crashes in multi-agent async deployments
• Silent data loss (tools registered to wrong registry instance)
• Intermittent failures that are hard to reproduce and debug

Expected Behavior

Per the stated principle: "Multi-agent + async safe by default" — all shared mutable state should be lock-protected, and async/sync boundaries should be handled correctly.

[github-actions]

@copilot Do a thorough triage and analysis of this issue:

1. Classification: Is this a bug, feature request, question, or documentation gap?
2. Severity: Critical / High / Medium / Low — explain why
3. Affected area: Which modules/files in the codebase are relevant?
4. Root cause analysis: Search the codebase and identify the likely root cause
5. Impact assessment: What else could be affected? Any related issues?
6. Suggested fix: Propose a concrete solution approach with file paths
7. Effort estimate: Small (hours) / Medium (day) / Large (days+)

[github-actions]

Claude finished @MervinPraison's task —— View job

---

Thread Safety Triage & Analysis Complete ✅

Classification: BUG - Multiple Critical Thread Safety Issues

Severity: HIGH

• Production crashes in multi-agent deployments
• Silent data corruption potential
• Core architectural component affected (tool registry singleton)
• Widespread async/threading usage (88 files use threading, 158+ use asyncio)

Affected Areas & Root Cause Analysis

1. Unprotected Global State (agents.py:33-35) - CONFIRMED

# NO thread safety - contrast with agent.py which HAS _server_lock  
_agents_server_started = {}
_agents_registered_endpoints = {}  
_agents_shared_apps = {}

• Root Cause: Missing threading.Lock() despite concurrent FastAPI server registration
• Pattern: agent.py:157 correctly uses _server_lock = threading.Lock() for identical use case
• Evidence: Lines 1617-1851 show concurrent modification during server startup

2. Tool Registry Singleton Race (tools/registry.py:256-261) - CONFIRMED

def get_registry() -> ToolRegistry:
    global _global_registry
    if _global_registry is None:        # ← Race condition here
        _global_registry = ToolRegistry()
    return _global_registry

• Root Cause: Classic check-then-act race condition
• Impact: Multiple threads can create separate registry instances → tools registered in one invisible to others
• Critical: Core infrastructure component used across entire codebase

3. AsyncIO Mixing (agent.py:5067) - CONFIRMED

decision = asyncio.run(backend.request_approval(request))  # ← RuntimeError in async context

• Root Cause: asyncio.run() cannot be called from within running event loop
• Context: Called from _check_tool_approval_sync() during async operations
• Impact: Production crashes during agent approval flows

4. Concurrent Dict Iteration/Modification (agent.py:8353-8384) - CONFIRMED

for tid, info in self._pending_approvals.items():  # ← Concurrent iteration
    # ... processing ...
    
for tid in completed_ids:
    del self._pending_approvals[tid]  # ← Concurrent modification

• Root Cause: No synchronization around dict read/write operations
• Impact: RuntimeError: dictionary changed size during iteration

Related Impact Assessment

High Risk Areas Found:

• 88+ files use threading across codebase - indicates heavy multi-threaded architecture
• 158+ files use asyncio - mixed async/sync patterns throughout
• Multiple similar patterns identified requiring review:
  • praisonaiagents/bus/bus.py - Event bus threading
  • praisonaiagents/storage/ - Storage backends threading
  • praisonaiagents/session/store.py - Session management

Existing Safety Patterns:

✅ agent.py:157 - Correct use of _server_lock for server state
✅ tests/unit/test_thread_safety.py - Thread safety test coverage exists
✅ examples/performance/thread_safety_example.py - Documentation of safe patterns

Suggested Fix Approach

Immediate (Critical Path):

1. Add locking to agents.py - Copy pattern from agent.py:157
2. Fix registry singleton - Implement double-checked locking
3. Replace asyncio.run() - Use await or detect event loop state
4. Protect pending approvals - Add asyncio.Lock() or ThreadSafe dict

Implementation Strategy:

# agents.py fix
_agents_lock = threading.Lock()
# Use: with _agents_lock: before all dict operations

# registry.py fix  
_registry_lock = threading.Lock()
def get_registry() -> ToolRegistry:
    global _global_registry
    if _global_registry is None:
        with _registry_lock:
            if _global_registry is None:
                _global_registry = ToolRegistry()
    return _global_registry

# agent.py async fix
try:
    loop = asyncio.get_running_loop()
    decision = await backend.request_approval(request)  
except RuntimeError:  # No running loop
    decision = asyncio.run(backend.request_approval(request))

Effort Estimate: MEDIUM (2-3 days)

• 4 specific fixes in core files
• Comprehensive testing required across multi-agent scenarios
• Potential need to audit similar patterns in other components
• Regression risk: moderate (core components affected)

Recommendation

Priority 1 - These are legitimate production-blocking bugs in a system that explicitly promises "multi-agent + async safe by default". The fixes are well-defined and follow existing patterns in the codebase.

---

[github-actions]

Claude finished @MervinPraison's task —— View job • claude/issue-1167-20260330-1659 • [Create PR ➔](https://github.com/MervinPraison/PraisonAI/compare/main...claude/issue-1167-20260330-1659?quick_pull=1&title=Issue%20%231167%3A%20Changes%20from%20Claude&body=This%20PR%20addresses%20issue%20%231167%0A%0AGenerated%20with%20%5BClaude%20Code%5D(https%3A%2F%2Fclaude.ai%2Fcode)

---

Thread Safety Analysis & Fix Implementation ✅

Todo List:

• Read AGENTS.md for repository guidelines
• Search codebase to confirm the 4 identified issues
• Implement fix for unprotected global dicts in agents/agents.py
• Implement fix for ToolRegistry singleton race condition
• Implement fix for asyncio.run() in async context
• Implement fix for unprotected _pending_approvals dict
• Run tests to verify fixes
• Commit changes and create PR

Issues Fixed ✅

1. agents.py:33-37 - Added _agents_lock = threading.Lock() and protected all access to global dicts with proper locking
2. tools/registry.py:258-266 - Implemented double-checked locking pattern for thread-safe singleton
3. agent.py:5067 - Fixed asyncio.run() in async context using event loop detection and ThreadPoolExecutor
4. agent.py:8349-8437 - Added asyncio.Lock() for _pending_approvals dict with proper async locking and deadlock prevention

Implementation Details

All fixes follow the existing patterns in the codebase and AGENTS.md guidelines for "multi-agent + async safe by default":

• Thread safety: Used threading.Lock() for sync operations and asyncio.Lock() for async operations
• Deadlock prevention: Tool execution happens outside the lock in check_pending_approvals()
• Backward compatibility: Added thread-safe async methods alongside existing sync properties
• Existing patterns: Followed agent.py:157 pattern for server lock usage

Pull Request Created

🔗 PR #1182: #1182

The fix ensures production stability for multi-agent deployments by preventing:

• Race conditions in server startup and tool registry
• RuntimeError crashes from nested event loop usage
• Dictionary iteration errors during concurrent modifications

---