Skip to content

Comments

Fix CapacityLimiter._pending_borrowers leak on non-Cancelled exceptions#3399

Closed
bysiber wants to merge 1 commit intopython-trio:mainfrom
bysiber:fix/capacity-limiter-pending-borrowers-leak
Closed

Fix CapacityLimiter._pending_borrowers leak on non-Cancelled exceptions#3399
bysiber wants to merge 1 commit intopython-trio:mainfrom
bysiber:fix/capacity-limiter-pending-borrowers-leak

Conversation

@bysiber
Copy link

@bysiber bysiber commented Feb 20, 2026

Problem

CapacityLimiter.acquire_on_behalf_of only cleans up _pending_borrowers when park() raises trio.Cancelled. If park() raises any other exception — such as KeyboardInterrupt delivered to a parked task via the abort mechanism — the task's entry stays in _pending_borrowers permanently.

The relevant code:

try:
    await self._lot.park()
except trio.Cancelled:
    self._pending_borrowers.pop(task)
    raise

When KeyboardInterrupt is delivered to the task, the abort function succeeds and the task is rescheduled with raise_cancel() which raises KeyboardInterrupt, not trio.Cancelled. The stale entry leaks.

Fix

Broadened the exception handler to BaseException and use pop(task, None) for safety against double-removal:

except BaseException:
    self._pending_borrowers.pop(task, None)
    raise

@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00000%. Comparing base (2fd138e) to head (16ec20f).

Additional details and impacted files
@@               Coverage Diff               @@
##                 main        #3399   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             128          128           
  Lines           19424        19424           
  Branches         1318         1318           
===============================================
  Hits            19424        19424           
Files with missing lines Coverage Δ
src/trio/_sync.py 100.00000% <100.00000%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

acquire_on_behalf_of only catches trio.Cancelled when cleaning up
_pending_borrowers.  If park() raises any other exception (e.g.
KeyboardInterrupt delivered via the abort mechanism), the task's
entry remains in _pending_borrowers permanently.  This is a slow
memory leak and can also cause _wake_waiters to encounter stale
entries.

Broaden the handler to BaseException and use pop with a default
to be safe against double-removal.
@bysiber bysiber force-pushed the fix/capacity-limiter-pending-borrowers-leak branch from 3f66165 to 16ec20f Compare February 20, 2026 06:55
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create an issue before fixing bugs like this. I'm fairly certain this is not actually an issue, so I will be closing this. I'd be happy to reopen this though.

await self._lot.park()
except trio.Cancelled:
self._pending_borrowers.pop(task)
except BaseException:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KI cannot be raised here because we are in a @enable_ki_protection function.

@A5rocks A5rocks closed this Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants