Potential for resource leak in `circuit_breaker.py` #8246

New issue

Open

opened 2026-04-13 06:20:07 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-13 06:20:07 +00:00

Owner

Metadata

File: src/cleveragents/core/circuit_breaker.py
Line: 152
Commit message: fix(core): move half-open permit decrement inside try block in circuit_breaker
Branch name: fix/core-circuit-breaker-half-open-permit-leak

Background and Context

In src/cleveragents/core/circuit_breaker.py, both the call and async_call methods decrement _half_open_permits before the try...except block. If any exception other than those caught by the try block is raised between the decrement and the entry into the try block (e.g. a KeyboardInterrupt, SystemExit, or any other BaseException subclass that is not Exception), the permit will be permanently lost.

While this window is extremely narrow and the scenario is unlikely in practice, it represents a latent correctness issue: the circuit breaker's half-open concurrency accounting can become permanently off by one, causing the breaker to reject more calls than it should during the half-open recovery phase. Over time — or under adversarial conditions — this could prevent the circuit from ever fully recovering.

Impacted component: core

Expected Behavior

_half_open_permits is decremented inside the try block in both call and async_call.
If an exception is raised before the decrement can occur, the permit is not consumed and the circuit breaker's accounting remains correct.
The finally block (which increments _half_open_permits back) only runs if the decrement was reached, preventing a double-increment.

Acceptance Criteria

In circuit_breaker.py, the _half_open_permits decrement in call is moved to be the first statement inside the try block.
In circuit_breaker.py, the _half_open_permits decrement in async_call is moved to be the first statement inside the try block.
The corresponding finally block (or equivalent restore logic) is verified to only run when the decrement was reached, so no double-increment occurs.
Existing unit tests for CircuitBreaker continue to pass.
New or updated tests cover the half-open permit accounting under exception conditions.
No regressions in nox / CI pipeline.

Subtasks

Review call at line 152 in src/cleveragents/core/circuit_breaker.py and map the full permit decrement/restore flow.
Review async_call for the same pattern.
Move the _half_open_permits -= 1 decrement to be the first statement inside the try block for both methods.
Verify the finally / restore path is still correct after the move (no double-increment on the happy path).
Update or add unit tests to cover the permit accounting under BaseException / KeyboardInterrupt injection.
Run nox locally to confirm all tests pass and coverage is maintained.
Update inline comments/docstrings to explain why the decrement is inside the try block.

Definition of Done

This issue should be closed when:

_half_open_permits is decremented inside the try block in both call and async_call.
The permit restore logic in finally (or equivalent) is verified correct — no double-increment on the happy path, no lost permit on the error path.
All existing tests pass and the new permit-accounting path is covered by tests.
The change is merged to the main branch via a reviewed PR.

Reproduction Steps: N/A — identified via static code review.

Proposed Fix: Move the decrement of _half_open_permits inside the try block. Example:

# Before (vulnerable):
self._half_open_permits -= 1
try:
    result = func(*args, **kwargs)
    ...
finally:
    self._half_open_permits += 1

# After (safe):
try:
    self._half_open_permits -= 1
    result = func(*args, **kwargs)
    ...
finally:
    self._half_open_permits += 1

This ensures that if any BaseException is raised before the decrement executes, the permit is never consumed and the finally block's increment does not produce a net gain.

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **File:** `src/cleveragents/core/circuit_breaker.py` - **Line:** 152 - **Commit message:** `fix(core): move half-open permit decrement inside try block in circuit_breaker` - **Branch name:** `fix/core-circuit-breaker-half-open-permit-leak` ## Background and Context In `src/cleveragents/core/circuit_breaker.py`, both the `call` and `async_call` methods decrement `_half_open_permits` **before** the `try...except` block. If any exception other than those caught by the `try` block is raised between the decrement and the entry into the `try` block (e.g. a `KeyboardInterrupt`, `SystemExit`, or any other `BaseException` subclass that is not `Exception`), the permit will be permanently lost. While this window is extremely narrow and the scenario is unlikely in practice, it represents a latent correctness issue: the circuit breaker's half-open concurrency accounting can become permanently off by one, causing the breaker to reject more calls than it should during the half-open recovery phase. Over time — or under adversarial conditions — this could prevent the circuit from ever fully recovering. **Impacted component:** `core` ## Expected Behavior - `_half_open_permits` is decremented **inside** the `try` block in both `call` and `async_call`. - If an exception is raised before the decrement can occur, the permit is not consumed and the circuit breaker's accounting remains correct. - The `finally` block (which increments `_half_open_permits` back) only runs if the decrement was reached, preventing a double-increment. ## Acceptance Criteria - [ ] In `circuit_breaker.py`, the `_half_open_permits` decrement in `call` is moved to be the first statement inside the `try` block. - [ ] In `circuit_breaker.py`, the `_half_open_permits` decrement in `async_call` is moved to be the first statement inside the `try` block. - [ ] The corresponding `finally` block (or equivalent restore logic) is verified to only run when the decrement was reached, so no double-increment occurs. - [ ] Existing unit tests for `CircuitBreaker` continue to pass. - [ ] New or updated tests cover the half-open permit accounting under exception conditions. - [ ] No regressions in `nox` / CI pipeline. ## Subtasks - [ ] Review `call` at line 152 in `src/cleveragents/core/circuit_breaker.py` and map the full permit decrement/restore flow. - [ ] Review `async_call` for the same pattern. - [ ] Move the `_half_open_permits -= 1` decrement to be the first statement inside the `try` block for both methods. - [ ] Verify the `finally` / restore path is still correct after the move (no double-increment on the happy path). - [ ] Update or add unit tests to cover the permit accounting under `BaseException` / `KeyboardInterrupt` injection. - [ ] Run `nox` locally to confirm all tests pass and coverage is maintained. - [ ] Update inline comments/docstrings to explain why the decrement is inside the `try` block. ## Definition of Done This issue should be closed when: 1. `_half_open_permits` is decremented inside the `try` block in both `call` and `async_call`. 2. The permit restore logic in `finally` (or equivalent) is verified correct — no double-increment on the happy path, no lost permit on the error path. 3. All existing tests pass and the new permit-accounting path is covered by tests. 4. The change is merged to the main branch via a reviewed PR. **Reproduction Steps:** N/A — identified via static code review. **Proposed Fix:** Move the decrement of `_half_open_permits` inside the `try` block. Example: ```python # Before (vulnerable): self._half_open_permits -= 1 try: result = func(*args, **kwargs) ... finally: self._half_open_permits += 1 # After (safe): try: self._half_open_permits -= 1 result = func(*args, **kwargs) ... finally: self._half_open_permits += 1 ``` This ensures that if any `BaseException` is raised before the decrement executes, the permit is never consumed and the `finally` block's increment does not produce a net gain. --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added this to the v3.2.0 milestone

2026-04-13 06:20:41 +00:00

HAL9000 commented

2026-04-13 06:37:59 +00:00

Author

Owner

[AUTO-EPIC] Epic Linkage

This issue is a child of Epic #8043 — M3 UAT Bug Resolution — Spec Alignment Backlog (v3.2.0).

The resource leak in circuit_breaker.py is a code quality/correctness issue that falls under the M3 UAT bug resolution scope. It should be resolved before v3.2.0 can be accepted.

Dependency direction: This issue (#8246) BLOCKS Epic #8043. Epic #8043 DEPENDS ON this issue being resolved.

Automated by CleverAgents Bot
Supervisor: Epic Planning | Agent: epic-planning-pool-supervisor

## [AUTO-EPIC] Epic Linkage This issue is a child of **Epic #8043** — M3 UAT Bug Resolution — Spec Alignment Backlog (v3.2.0). The resource leak in `circuit_breaker.py` is a code quality/correctness issue that falls under the M3 UAT bug resolution scope. It should be resolved before v3.2.0 can be accepted. **Dependency direction**: This issue (#8246) BLOCKS Epic #8043. Epic #8043 DEPENDS ON this issue being resolved. --- **Automated by CleverAgents Bot** Supervisor: Epic Planning | Agent: epic-planning-pool-supervisor

HAL9000 referenced this issue

2026-04-13 07:10:16 +00:00

[AUTO-EPIC] Status: Epic Planning Supervisor — Cycle 10 #8273

HAL9000 referenced this issue

2026-04-13 08:54:14 +00:00

[AUTO-BUG-POOL] Status: Bug Hunt Pool Status Update (Cycle 1) #8327