[events/EventBus] EventBus protocol and implementations lack unsubscribe() — handler references cause memory leaks in long-running processes [In Review] #10356

Open
opened 2026-04-18 09:08:46 +00:00 by HAL9000 · 8 comments
Owner

Metadata

  • Commit: fix(events): add unsubscribe() to EventBus protocol and implementations
  • Branch: fix/events-eventbus-unsubscribe

Background and Context

The EventBus protocol and both implementations (ReactiveEventBus, LoggingEventBus) in src/cleveragents/infrastructure/events/ lack an unsubscribe() method. Once a handler is registered via subscribe(), it cannot be removed. In long-running processes, this causes memory leaks because handler callables (bound methods) hold strong references to their owning objects, preventing garbage collection even after the owning component is shut down.

Expected Behavior

The EventBus protocol should include an unsubscribe() method, and both implementations should support it:

def unsubscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None:
    """Remove a previously registered handler for event_type.
    
    If the handler was not registered, this is a no-op.
    """

Acceptance Criteria

  • EventBus protocol includes unsubscribe() method
  • ReactiveEventBus.unsubscribe() removes the handler from _subscriptions
  • LoggingEventBus.unsubscribe() removes the handler from _subscriptions
  • Unsubscribed handlers are not called on subsequent emit() calls
  • Unsubscribing a non-registered handler is a no-op (no exception)
  • TDD test (see blocked-by issue #10354) passes after fix
  • nox passes with coverage ≥ 97%

Subtasks

  • Add unsubscribe() to EventBus protocol in protocol.py
  • Implement unsubscribe() in ReactiveEventBus
  • Implement unsubscribe() in LoggingEventBus
  • Add tests verifying unsubscription behavior
  • Add test verifying no strong references remain after unsubscribe
  • Run nox to confirm coverage ≥ 97%

Definition of Done

This issue is closed when:

  1. unsubscribe() is implemented in the protocol and both implementations
  2. The TDD test (blocked-by issue #10354) passes
  3. nox passes with coverage ≥ 97%
  4. A PR is reviewed and merged to main

Current Behaviour

# src/cleveragents/infrastructure/events/protocol.py
class EventBus(Protocol):
    def emit(self, event: DomainEvent) -> None: ...
    def subscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None: ...
    # No unsubscribe() method

# src/cleveragents/infrastructure/events/reactive.py
class ReactiveEventBus:
    def subscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None:
        self._subscriptions.setdefault(event_type, []).append(handler)
    # No unsubscribe() method — handler is stored forever

Calling bus.unsubscribe(EventType.PLAN_CREATED, my_handler) raises AttributeError: 'ReactiveEventBus' object has no attribute 'unsubscribe'.

Impact

  1. Memory leaks: When a service subscribes to events and is later shut down, the handler (a bound method) holds a reference to the service object. The _subscriptions dict in the bus holds a reference to the handler. This creates a reference cycle that prevents the service from being garbage collected.

  2. Stale handler invocations: After a component is "destroyed" (its reference deleted by the caller), its handler continues to be called on every emit(). This can cause:

    • Errors if the handler accesses resources that have been cleaned up
    • Unexpected side effects from a "dead" component
    • Difficult-to-debug behavior in tests that reuse bus instances
  3. No cleanup path: There is no way to clean up subscriptions when a component is shut down, making it impossible to write leak-free code that uses the event bus.

Fix

  1. Add unsubscribe() to EventBus protocol in protocol.py
  2. Implement in ReactiveEventBus:
def unsubscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None:
    handlers = self._subscriptions.get(event_type, [])
    try:
        handlers.remove(handler)
    except ValueError:
        pass
  1. Implement identically in LoggingEventBus
  2. Consider using weakref.WeakMethod for bound method handlers to enable automatic cleanup

References

  • src/cleveragents/infrastructure/events/protocol.pyEventBus protocol
  • src/cleveragents/infrastructure/events/reactive.pyReactiveEventBus
  • src/cleveragents/infrastructure/events/logging_bus.pyLoggingEventBus

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Metadata - **Commit**: `fix(events): add unsubscribe() to EventBus protocol and implementations` - **Branch**: `fix/events-eventbus-unsubscribe` ## Background and Context The `EventBus` protocol and both implementations (`ReactiveEventBus`, `LoggingEventBus`) in `src/cleveragents/infrastructure/events/` lack an `unsubscribe()` method. Once a handler is registered via `subscribe()`, it cannot be removed. In long-running processes, this causes memory leaks because handler callables (bound methods) hold strong references to their owning objects, preventing garbage collection even after the owning component is shut down. ## Expected Behavior The `EventBus` protocol should include an `unsubscribe()` method, and both implementations should support it: ```python def unsubscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None: """Remove a previously registered handler for event_type. If the handler was not registered, this is a no-op. """ ``` ## Acceptance Criteria - [ ] `EventBus` protocol includes `unsubscribe()` method - [ ] `ReactiveEventBus.unsubscribe()` removes the handler from `_subscriptions` - [ ] `LoggingEventBus.unsubscribe()` removes the handler from `_subscriptions` - [ ] Unsubscribed handlers are not called on subsequent `emit()` calls - [ ] Unsubscribing a non-registered handler is a no-op (no exception) - [ ] TDD test (see blocked-by issue #10354) passes after fix - [ ] `nox` passes with coverage ≥ 97% ## Subtasks - [ ] Add `unsubscribe()` to `EventBus` protocol in `protocol.py` - [ ] Implement `unsubscribe()` in `ReactiveEventBus` - [ ] Implement `unsubscribe()` in `LoggingEventBus` - [ ] Add tests verifying unsubscription behavior - [ ] Add test verifying no strong references remain after unsubscribe - [ ] Run `nox` to confirm coverage ≥ 97% ## Definition of Done This issue is closed when: 1. `unsubscribe()` is implemented in the protocol and both implementations 2. The TDD test (blocked-by issue #10354) passes 3. `nox` passes with coverage ≥ 97% 4. A PR is reviewed and merged to `main` --- ## Current Behaviour ```python # src/cleveragents/infrastructure/events/protocol.py class EventBus(Protocol): def emit(self, event: DomainEvent) -> None: ... def subscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None: ... # No unsubscribe() method # src/cleveragents/infrastructure/events/reactive.py class ReactiveEventBus: def subscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None: self._subscriptions.setdefault(event_type, []).append(handler) # No unsubscribe() method — handler is stored forever ``` Calling `bus.unsubscribe(EventType.PLAN_CREATED, my_handler)` raises `AttributeError: 'ReactiveEventBus' object has no attribute 'unsubscribe'`. ## Impact 1. **Memory leaks**: When a service subscribes to events and is later shut down, the handler (a bound method) holds a reference to the service object. The `_subscriptions` dict in the bus holds a reference to the handler. This creates a reference cycle that prevents the service from being garbage collected. 2. **Stale handler invocations**: After a component is "destroyed" (its reference deleted by the caller), its handler continues to be called on every `emit()`. This can cause: - Errors if the handler accesses resources that have been cleaned up - Unexpected side effects from a "dead" component - Difficult-to-debug behavior in tests that reuse bus instances 3. **No cleanup path**: There is no way to clean up subscriptions when a component is shut down, making it impossible to write leak-free code that uses the event bus. ## Fix 1. Add `unsubscribe()` to `EventBus` protocol in `protocol.py` 2. Implement in `ReactiveEventBus`: ```python def unsubscribe(self, event_type: EventType, handler: Callable[[DomainEvent], None]) -> None: handlers = self._subscriptions.get(event_type, []) try: handlers.remove(handler) except ValueError: pass ``` 3. Implement identically in `LoggingEventBus` 4. Consider using `weakref.WeakMethod` for bound method handlers to enable automatic cleanup ## References - `src/cleveragents/infrastructure/events/protocol.py` — `EventBus` protocol - `src/cleveragents/infrastructure/events/reactive.py` — `ReactiveEventBus` - `src/cleveragents/infrastructure/events/logging_bus.py` — `LoggingEventBus` --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor
HAL9000 added this to the v3.2.0 milestone 2026-04-18 09:11:19 +00:00
Author
Owner

[GROOMED] Quality Analysis Complete

Issue Validity: VALID & ACTIONABLE

This is a legitimate, well-documented bug report about a critical missing feature in the EventBus infrastructure:

  • Problem: Missing unsubscribe() method causes memory leaks in long-running processes
  • Root Cause: Handler references (bound methods) are stored indefinitely in _subscriptions dict
  • Impact: Prevents garbage collection of service objects, causes stale handler invocations

Triage Assessment

Aspect Status Details
Labels Complete State/Unverified, Type/Bug, Priority/Critical
Milestone Assigned v3.2.0 (M3: Decisions + Validations + Invariants)
Assignee Pending Not yet assigned (awaiting team decision)
Epic ℹ️ N/A Standalone infrastructure fix

Quality Checklist

  • Issue is valid and actionable
  • All required labels applied (State/, Type/, Priority/)
  • Appropriate milestone assigned (v3.2.0)
  • Clear acceptance criteria defined
  • Subtasks enumerated
  • Definition of Done specified
  • Code examples provided
  • Related files referenced
  • No orphaned issue (assigned to milestone)
  1. Assign to developer - Ready for implementation
  2. Create related TDD test issue - Issue mentions blocked-by #10354
  3. Implementation scope - Affects 3 files:
    • src/cleveragents/infrastructure/events/protocol.py
    • src/cleveragents/infrastructure/events/reactive.py
    • src/cleveragents/infrastructure/events/logging_bus.py

Notes

  • Issue is well-structured with clear problem statement and solution outline
  • Includes consideration of weakref.WeakMethod for automatic cleanup
  • Coverage requirement (≥97%) aligns with project standards
  • Ready for State/Verified transition once assigned

Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-pool-supervisor

## [GROOMED] Quality Analysis Complete ### Issue Validity: ✅ VALID & ACTIONABLE This is a legitimate, well-documented bug report about a critical missing feature in the EventBus infrastructure: - **Problem**: Missing `unsubscribe()` method causes memory leaks in long-running processes - **Root Cause**: Handler references (bound methods) are stored indefinitely in `_subscriptions` dict - **Impact**: Prevents garbage collection of service objects, causes stale handler invocations ### Triage Assessment | Aspect | Status | Details | |--------|--------|----------| | **Labels** | ✅ Complete | State/Unverified, Type/Bug, Priority/Critical | | **Milestone** | ✅ Assigned | v3.2.0 (M3: Decisions + Validations + Invariants) | | **Assignee** | ⏳ Pending | Not yet assigned (awaiting team decision) | | **Epic** | ℹ️ N/A | Standalone infrastructure fix | ### Quality Checklist - ✅ Issue is valid and actionable - ✅ All required labels applied (State/, Type/, Priority/) - ✅ Appropriate milestone assigned (v3.2.0) - ✅ Clear acceptance criteria defined - ✅ Subtasks enumerated - ✅ Definition of Done specified - ✅ Code examples provided - ✅ Related files referenced - ✅ No orphaned issue (assigned to milestone) ### Recommended Next Steps 1. **Assign to developer** - Ready for implementation 2. **Create related TDD test issue** - Issue mentions blocked-by #10354 3. **Implementation scope** - Affects 3 files: - `src/cleveragents/infrastructure/events/protocol.py` - `src/cleveragents/infrastructure/events/reactive.py` - `src/cleveragents/infrastructure/events/logging_bus.py` ### Notes - Issue is well-structured with clear problem statement and solution outline - Includes consideration of `weakref.WeakMethod` for automatic cleanup - Coverage requirement (≥97%) aligns with project standards - Ready for State/Verified transition once assigned --- **Automated by CleverAgents Bot** Supervisor: Grooming | Agent: grooming-pool-supervisor
HAL9000 self-assigned this 2026-04-28 05:01:36 +00:00
Author
Owner

Implementation Attempt — Tier 3: sonnet — Success

Implemented unsubscribe() method in the EventBus protocol and both implementations.

Files changed:

  • src/cleveragents/infrastructure/events/protocol.py — Added unsubscribe() to EventBus Protocol
  • src/cleveragents/infrastructure/events/reactive.py — Implemented unsubscribe() with contextlib.suppress(ValueError)
  • src/cleveragents/infrastructure/events/logging_bus.py — Identical implementation
  • features/event_bus.feature — Added 7 BDD scenarios tagged @tdd_issue @tdd_issue_10354
  • features/steps/event_bus_steps.py — Added step definitions
  • CHANGELOG.md — Added fix entry

Quality gate status: lint pass, typecheck pass

PR created: #10887


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor

**Implementation Attempt** — Tier 3: sonnet — Success Implemented unsubscribe() method in the EventBus protocol and both implementations. Files changed: - src/cleveragents/infrastructure/events/protocol.py — Added unsubscribe() to EventBus Protocol - src/cleveragents/infrastructure/events/reactive.py — Implemented unsubscribe() with contextlib.suppress(ValueError) - src/cleveragents/infrastructure/events/logging_bus.py — Identical implementation - features/event_bus.feature — Added 7 BDD scenarios tagged @tdd_issue @tdd_issue_10354 - features/steps/event_bus_steps.py — Added step definitions - CHANGELOG.md — Added fix entry Quality gate status: lint pass, typecheck pass PR created: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/10887 --- Automated by CleverAgents Bot Supervisor: Implementation | Agent: task-implementor
Owner

Implementation Attempt — Tier default: qwen-small — Success

Implemented unsubscribe() method in the EventBus Protocol and both implementations (ReactiveEventBus, LoggingEventBus).

Files changed:

  • src/cleveragents/infrastructure/events/protocol.py — Added unsubscribe() to EventBus Protocol with docstring
  • src/cleveragents/infrastructure/events/reactive.py — Implemented unsubscribe() with contextlib.suppress(ValueError)
  • src/cleveragents/infrastructure/events/logging_bus.py — Added contextlib import + identical unsubscribe implementation
  • features/event_bus.feature — Added 5 BDD scenarios for unsubscribe behavior
  • features/steps/event_bus_steps.py — Added step definitions for unsubscribe scenarios
  • CHANGELOG.md — Added fix entry under [Unreleased]
  • CONTRIBUTORS.md — Updated contribution detail

Quality gate status: lint pass, typecheck pass, unit tests pass (410 scenarios passed, 0 failed; 5 pre-existing errors in unrelated feature files), integration and e2e CI will verify on merge.

PR created: https://git.cleveragents.com/cleveragents/cleveragents-core/pulls/11135


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor

**Implementation Attempt** — Tier default: qwen-small — Success Implemented `unsubscribe()` method in the EventBus Protocol and both implementations (ReactiveEventBus, LoggingEventBus). Files changed: - src/cleveragents/infrastructure/events/protocol.py — Added unsubscribe() to EventBus Protocol with docstring - src/cleveragents/infrastructure/events/reactive.py — Implemented unsubscribe() with contextlib.suppress(ValueError) - src/cleveragents/infrastructure/events/logging_bus.py — Added contextlib import + identical unsubscribe implementation - features/event_bus.feature — Added 5 BDD scenarios for unsubscribe behavior - features/steps/event_bus_steps.py — Added step definitions for unsubscribe scenarios - CHANGELOG.md — Added fix entry under [Unreleased] - CONTRIBUTORS.md — Updated contribution detail Quality gate status: lint pass, typecheck pass, unit tests pass (410 scenarios passed, 0 failed; 5 pre-existing errors in unrelated feature files), integration and e2e CI will verify on merge. PR created: https://git.cleveragents.com/cleveragents/cleveragents-core/pulls/11135 --- Automated by CleverAgents Bot Supervisor: Implementation | Agent: task-implementor
Owner

Referenced in PR #11197

Referenced in [PR #11197](https://git.cleverthis.com/cleveragents/cleveragents-core/pull/11197)
freemo changed title from [events/EventBus] EventBus protocol and implementations lack unsubscribe() — handler references cause memory leaks in long-running processes to [events/EventBus] EventBus protocol ... [In Review] 2026-05-13 14:00:01 +00:00
Owner

Closed by PR #11197.

Closed by [PR #11197](https://git.cleverthis.com/cleveragents/cleveragents-core/pull/11197).
Owner

State updated: In Review. Milestone: v3.2.0 (id=105).

State updated: In Review. Milestone: v3.2.0 (id=105).
freemo changed title from [events/EventBus] EventBus protocol ... [In Review] to [events/EventBus] EventBus protocol and implementations lack unsubscribe() — handler references cause memory leaks in long-running processes [In Review] 2026-05-13 14:18:06 +00:00
Owner

Note from PR #11197 review:

PR #11197 was submitted for this issue but does NOT implement the described fix. Instead of adding unsubscribe() to EventBus, it:

  • Removes functionality (ReactiveEventBus._closed, context manager, invariant propagation)
  • Deletes tests (causing CI failures and coverage drop below 97%)
  • Contains multiple unrelated changes violating atomicity

The PR has been marked Request Changes and cannot be merged until resubmitted with correct scope. The author should:

  1. Create a new branch focused ONLY on adding unsubscribe() protocol method
  2. Implement the fix in both ReactiveEventBus and LoggingEventBus
  3. Add BDD tests for the new functionality (not delete existing tests)
  4. Submit as a separate, atomic PR
**Note from PR #11197 review:** PR #11197 was submitted for this issue but does NOT implement the described fix. Instead of adding `unsubscribe()` to EventBus, it: - Removes functionality (`ReactiveEventBus._closed`, context manager, invariant propagation) - Deletes tests (causing CI failures and coverage drop below 97%) - Contains multiple unrelated changes violating atomicity The PR has been marked `Request Changes` and cannot be merged until resubmitted with correct scope. The author should: 1. Create a new branch focused ONLY on adding `unsubscribe()` protocol method 2. Implement the fix in both `ReactiveEventBus` and `LoggingEventBus` 3. Add BDD tests for the new functionality (not delete existing tests) 4. Submit as a separate, atomic PR
Author
Owner

[GROOMING] Orphaned Hierarchy Detected

This issue (#10356) does not have a parent Epic dependency link. Per CleverThis guidelines, every regular issue must link to a parent Epic (the issue should BLOCK its parent Epic).

Action required: The issue author or maintainer should link this issue to the appropriate parent Epic via the dependencies API.

Note: Automated grooming agent was unable to create the dependency link due to an internal tracker error. This needs manual attention.


Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-worker

## [GROOMING] Orphaned Hierarchy Detected This issue (#10356) does not have a parent Epic dependency link. Per CleverThis guidelines, every regular issue must link to a parent Epic (the issue should BLOCK its parent Epic). **Action required:** The issue author or maintainer should link this issue to the appropriate parent Epic via the dependencies API. Note: Automated grooming agent was unable to create the dependency link due to an internal tracker error. This needs manual attention. --- Automated by CleverAgents Bot Supervisor: Grooming | Agent: grooming-worker
Sign in to join this conversation.
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#10356
No description provided.