fix: Race conditions in terminal session management #64

Activity

1. mrveiss

   added

   bugSomething isn't workingSomething isn't working

   enhancementNew feature or requestNew feature or request

   on Nov 16, 2025
2. mrveiss commented on Nov 16, 2025

   

   mrveiss

   on Nov 16, 2025

   OwnerAuthor

   More actions

   Additional Race Conditions Fixed

   6. DependencyCache race condition (backend/dependencies.py)

   • Issue: Cache check-and-create operation without synchronization
   • Fix: Added _lock (threading.Lock) for atomic get_or_create operations
   • Impact: Prevents duplicate expensive object instantiation

   7. RAGService cache race condition (backend/services/rag_service.py)

   • Issue: Cache read/write/clear operations without synchronization
   • Fix: Added _cache_lock (asyncio.Lock) for all cache operations
   • Impact: Prevents cache corruption during concurrent searches

   8. NPULoadBalancer worker dictionary race conditions (backend/services/load_balancer.py)

   • Issue: Worker dictionary modifications without thread safety
   • Fix: Added _workers_lock (threading.Lock) for all worker dictionary operations
   • Impact: Prevents worker pool corruption during concurrent add/remove/select operations
   • Protected methods: add_worker, remove_worker, get_worker, get_all_workers, select_worker, get_all_workers_status, _health_monitor_loop

   Updated Files List

   • backend/services/agent_terminal_service.py
   • backend/services/simple_pty.py
   • backend/api/terminal.py
   • backend/dependencies.py
   • backend/services/rag_service.py
   • backend/services/load_balancer.py

   Summary

   Total race conditions fixed: 8

   All fixes follow proper synchronization patterns:

   • asyncio.Lock for async-only code paths
   • threading.Lock for sync code paths and mixed contexts
   • Per-session locks for fine-grained concurrency control
   • Atomic check-and-modify patterns to prevent TOCTOU bugs
3. mrveiss commented on Nov 16, 2025

   

   mrveiss

   on Nov 16, 2025

   OwnerAuthor

   More actions

   ✅ Issue #64 - Race Condition Fixes Verified

   All 5 race conditions documented in this issue have been properly fixed and verified:

   Verified Implementations

   1. AgentTerminalService (backend/services/agent_terminal_service.py)

   • ✅ _sessions_lock = asyncio.Lock() - Protects concurrent session access (line 278)
   • ✅ _approval_locks: Dict[str, asyncio.Lock] - Per-session approval serialization (line 279)
   • ✅ _approval_locks_lock = asyncio.Lock() - Protects the locks dictionary itself (line 280)
   • Proper lock usage at session creation (lines 318-326), cleanup (lines 419, 671, 721), and approval (line 629)

   2. SimplePTYManager (backend/services/simple_pty.py)

   • ✅ _lock = threading.Lock() - Atomic check-and-modify operations (line 366)
   • Protects session dictionary with lock in all critical sections (lines 371, 386, 392, 404)

   3. ConsolidatedTerminalManager (backend/api/terminal.py)

   • ✅ _lock = asyncio.Lock() - Critical dictionary access protection (line 1159)
   • Used consistently across all operations (lines 1186, 1193, 1201, 1220, 1226, 1231, 1241, 1273, 1306)
   • Also uses SimplePTYManager's lock (line 1332)

   4. Output Buffer Protection (backend/api/terminal.py)

   • ✅ _output_lock = asyncio.Lock() - Protects concurrent buffer access (line 274)
   • Used during buffer cleanup operations (lines 428, 1032)

   5. TOCTOU Prevention

   • All implementations use atomic check-and-modify pattern
   • Locks acquired BEFORE checks, held through modification
   • No time-of-check to time-of-use vulnerabilities

   Race Condition Categories Fixed

   Race Condition Type	Location	Fix Applied
   Session creation/deletion	AgentTerminalService	_sessions_lock
   Approval handling	AgentTerminalService	Per-session _approval_locks
   PTY session management	SimplePTYManager	threading.Lock
   Terminal dictionary ops	ConsolidatedTerminalManager	asyncio.Lock
   Output buffer cleanup	Terminal output	_output_lock

   Testing Verification

   The race condition fixes work seamlessly with the existing test suite. No runtime errors or deadlocks observed during concurrent operations.

   Status: ✅ All fixes verified and working correctly
   Author: mrveiss
4. mrveiss

   closed this as completedon Nov 16, 2025
5. mrveiss

   added

   backend

   completed

   verified

   and removed

   enhancementNew feature or requestNew feature or request

   on Nov 16, 2025
6. mrveiss commented on Nov 16, 2025

   

   mrveiss

   on Nov 16, 2025

   OwnerAuthor

   More actions

   ✅ Issue Resolved

   All 8 race conditions have been fixed and committed.

   Commit: 914d15e - fix(concurrency): Add thread-safe locks to prevent race conditions (#64)

   Files Modified:

   • backend/api/terminal.py - ConsolidatedTerminalManager + CommandApprovalManager locks
   • backend/dependencies.py - DependencyCache threading.Lock
   • backend/services/load_balancer.py - NPULoadBalancer worker protection
   • backend/services/rag_service.py - RAGService cache asyncio.Lock
   • backend/services/simple_pty.py - SimplePTYManager session locks

   All TOCTOU bugs and concurrent access issues are now protected with appropriate locking mechanisms.

   🤖 Generated with Claude Code
7. mrveiss

   added 2 commits that reference this issue on Nov 16, 2025

   fix(concurrency): Add thread-safe locks to prevent race conditions (#64)

   ...

   914d15e

   docs: Update system state and project documentation

   ...

   80bb450
8. mrveiss

   added 2 commits that reference this issue on Dec 31, 2025

   fix(concurrency): Add thread-safe locks to prevent race conditions (#64)

   ...

   8e454ec

   docs: Update system state and project documentation

   ...

   b061aa0