LoggingEventBus lacks thread safety despite potential concurrent use #10553

New issue

Open

opened 2026-04-18 17:12:38 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-18 17:12:38 +00:00

Owner

Metadata

Commit: Latest commit in current branch
Branch: main

Background and Context

The LoggingEventBus class in src/cleveragents/infrastructure/events/logging_bus.py is explicitly documented as "designed for single-threaded use" (lines 34-36), but there is no enforcement mechanism to prevent concurrent access from multiple threads. This creates a silent failure mode where race conditions could occur if the event bus is used in multi-threaded contexts, such as concurrent plan execution or parallel subplan processing.

The documentation states:

"""Thread safety: This implementation is designed for single-threaded use.
If concurrent access from multiple threads is required, callers must
provide external synchronization.

However, the _subscriptions dictionary (line 47) is accessed and modified without any locking mechanism:

self._subscriptions: dict[EventType, list[Callable[[DomainEvent], None]]] = {}

Methods like emit() and subscribe() modify this dictionary without synchronization, creating potential race conditions.

Expected Behavior

The LoggingEventBus should either:

Enforce thread safety by adding proper synchronization (e.g., threading.RLock) to protect concurrent access, OR
Enforce single-threaded usage by detecting and raising an error when accessed from multiple threads

The chosen approach should be documented clearly and consistently enforced throughout the codebase.

Acceptance Criteria

Thread safety is either enforced with locks OR single-threaded usage is validated with runtime checks
All dictionary access in _subscriptions is protected by the chosen synchronization mechanism
Methods emit(), subscribe(), and unsubscribe() are thread-safe or validated for single-threaded use
Documentation is updated to reflect the chosen approach
Unit tests verify thread safety or single-threaded enforcement
No race conditions occur when the event bus is used in concurrent scenarios

Subtasks

Analyze current usage patterns of LoggingEventBus to determine if multi-threaded access is expected
Decide between Option 1 (thread safety with locks) or Option 2 (enforce single-threaded usage)
Implement chosen solution with proper synchronization or validation
Add comprehensive unit tests for concurrent access scenarios
Update class documentation to clearly state thread safety guarantees
Review all call sites to ensure compliance with chosen approach

Definition of Done

Implementation is complete and tested
All acceptance criteria are met
Code review approved
Tests pass with coverage >= 97%
Documentation updated
No race conditions detected in concurrent scenarios

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata **Commit:** Latest commit in current branch **Branch:** main ## Background and Context The `LoggingEventBus` class in `src/cleveragents/infrastructure/events/logging_bus.py` is explicitly documented as "designed for single-threaded use" (lines 34-36), but there is no enforcement mechanism to prevent concurrent access from multiple threads. This creates a silent failure mode where race conditions could occur if the event bus is used in multi-threaded contexts, such as concurrent plan execution or parallel subplan processing. The documentation states: ```python """Thread safety: This implementation is designed for single-threaded use. If concurrent access from multiple threads is required, callers must provide external synchronization. ``` However, the `_subscriptions` dictionary (line 47) is accessed and modified without any locking mechanism: ```python self._subscriptions: dict[EventType, list[Callable[[DomainEvent], None]]] = {} ``` Methods like `emit()` and `subscribe()` modify this dictionary without synchronization, creating potential race conditions. ## Expected Behavior The `LoggingEventBus` should either: 1. **Enforce thread safety** by adding proper synchronization (e.g., `threading.RLock`) to protect concurrent access, OR 2. **Enforce single-threaded usage** by detecting and raising an error when accessed from multiple threads The chosen approach should be documented clearly and consistently enforced throughout the codebase. ## Acceptance Criteria - [ ] Thread safety is either enforced with locks OR single-threaded usage is validated with runtime checks - [ ] All dictionary access in `_subscriptions` is protected by the chosen synchronization mechanism - [ ] Methods `emit()`, `subscribe()`, and `unsubscribe()` are thread-safe or validated for single-threaded use - [ ] Documentation is updated to reflect the chosen approach - [ ] Unit tests verify thread safety or single-threaded enforcement - [ ] No race conditions occur when the event bus is used in concurrent scenarios ## Subtasks - [ ] Analyze current usage patterns of `LoggingEventBus` to determine if multi-threaded access is expected - [ ] Decide between Option 1 (thread safety with locks) or Option 2 (enforce single-threaded usage) - [ ] Implement chosen solution with proper synchronization or validation - [ ] Add comprehensive unit tests for concurrent access scenarios - [ ] Update class documentation to clearly state thread safety guarantees - [ ] Review all call sites to ensure compliance with chosen approach ## Definition of Done - Implementation is complete and tested - All acceptance criteria are met - Code review approved - Tests pass with coverage >= 97% - Documentation updated - No race conditions detected in concurrent scenarios --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added the

labels

2026-04-18 17:13:28 +00:00