- Enhance room_broadcast() reference counting:
* Check client state (connected, show_help, command_output) before rendering
* Perform state check while holding client ref_lock
* Prevents rendering to disconnected/invalid clients
* Ensures safe cleanup when ref count reaches zero
- Fix tui_render_screen() message array TOCTOU:
* Acquire all data (online count, message count, messages) in single lock
* Create snapshot of messages to display
* Calculate message range while holding lock
* Render from snapshot without holding lock
* Prevents inconsistencies from concurrent message additions
* Eliminates race between two separate lock acquisitions
- Fix handle_key() scroll position TOCTOU:
* Get message count atomically when calculating scroll bounds
* Calculate max_scroll properly accounting for message height
* Apply consistent bounds checking for 'j' (down) and 'G' (bottom)
* Prevents out-of-bounds access from concurrent message changes
These changes address:
- Race condition in broadcast rendering to disconnecting clients
- TOCTOU between message count read and message access
- Scroll position bounds check race conditions
Prevents:
- Use-after-free in client cleanup
- Array out-of-bounds access
- Inconsistent UI rendering
- Crashes from concurrent message list modifications
Improves thread safety without introducing deadlocks by:
- Using snapshot approach to avoid long lock holds
- Acquiring data in consistent lock order
- Minimizing critical sections
Fixes three critical bugs that caused crashes after long-running:
1. Use-after-free race condition in room_broadcast()
- Added reference counting to client_t structure
- Increment ref_count before using client outside lock
- Decrement and free only when ref_count reaches 0
- Prevents accessing freed client memory during broadcast
2. strtok() data corruption in tui_render_command_output()
- strtok() modifies original string by replacing delimiters
- Now use a local copy before calling strtok()
- Prevents corruption of client->command_output
3. Improved handle_key() consistency
- Return bool to indicate if key was consumed
- Fixes issue where mode-switch keys were processed twice
Thread safety changes:
- Added client->ref_count and client->ref_lock
- Added client_release() for safe cleanup
- room_broadcast() now properly increments/decrements refs
This fixes the primary cause of crashes during extended operation.
