Skip to content

Commit d107fc1

Browse files
committed
Fix scoped resource leak on cancellation in exit-stack cleanup
clean_exit_stack and async_clean_exit_stack caught only `except Exception` while iterating the exit stack. asyncio.CancelledError (and KeyboardInterrupt / SystemExit) are BaseException, not Exception, so when a generator's teardown re-raised the exception being unwound it escaped the loop early, skipping the remaining generators and exit_stack.clear(). On a cancelled `async with container.enter_scope()` block (asyncio.wait_for timeout, task cancellation) every earlier-registered scoped resource leaked, matching neither the documented behaviour nor contextlib.AsyncExitStack. Add an `except BaseException` branch to both loops: swallow the re-raised exc_val so cleanup of the remaining generators continues (and the stack is cleared), while propagating any genuinely new BaseException. New BaseExceptions are not appended to the errors list since ContainerCloseError subclasses ExceptionGroup on 3.11+, which rejects non-Exception members. Fixes #141
1 parent 3d90784 commit d107fc1

2 files changed

Lines changed: 90 additions & 2 deletions

File tree

test/unit/test_exit_stack.py

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
1+
import asyncio
12
import re
23
from collections.abc import AsyncGenerator, Generator
34

45
import pytest
56
from wireup.errors import WireupError
6-
from wireup.ioc._exit_stack import clean_exit_stack
7+
from wireup.ioc._exit_stack import async_clean_exit_stack, clean_exit_stack
8+
9+
10+
class _Cancelled(BaseException):
11+
"""Stand-in for a BaseException (not Exception) re-raised during teardown.
12+
13+
Mirrors asyncio.CancelledError / KeyboardInterrupt / SystemExit without
14+
tripping pytest's session-level handling of the latter two.
15+
"""
716

817

918
def test_clean_exit_stack_with_sync_generators() -> None:
@@ -46,3 +55,66 @@ async def gen_1() -> AsyncGenerator[None, None]:
4655
),
4756
):
4857
clean_exit_stack([(g1, True)])
58+
59+
60+
def test_clean_exit_stack_continues_teardown_when_unwinding_base_exception() -> None:
61+
# KeyboardInterrupt / SystemExit are BaseException, not Exception. When one is
62+
# being unwound, every generator must still be torn down and the stack cleared.
63+
teardowns: list[str] = []
64+
65+
def make(name: str) -> Generator[None, None, None]:
66+
try:
67+
yield
68+
finally:
69+
teardowns.append(name)
70+
71+
g1 = make("first")
72+
next(g1)
73+
g2 = make("second")
74+
next(g2)
75+
76+
exit_stack = [(g1, False), (g2, False)]
77+
clean_exit_stack(exit_stack, exc_val=_Cancelled())
78+
79+
assert teardowns == ["second", "first"]
80+
assert exit_stack == []
81+
82+
83+
async def test_async_clean_exit_stack_continues_teardown_on_cancellation() -> None:
84+
# Regression: when an `async with container.enter_scope()` block is cancelled
85+
# (asyncio.wait_for timeout, task.cancel()), the CancelledError is re-raised by
86+
# each generator's teardown. Cleanup must not abort early and leak the rest.
87+
teardowns: list[str] = []
88+
89+
async def make(name: str) -> AsyncGenerator[None, None]:
90+
try:
91+
yield
92+
finally:
93+
teardowns.append(name)
94+
95+
g1 = make("first")
96+
await g1.__anext__()
97+
g2 = make("second")
98+
await g2.__anext__()
99+
100+
exit_stack = [(g1, True), (g2, True)]
101+
await async_clean_exit_stack(exit_stack, exc_val=asyncio.CancelledError())
102+
103+
assert teardowns == ["second", "first"]
104+
assert exit_stack == []
105+
106+
107+
async def test_async_clean_exit_stack_propagates_new_base_exception_from_teardown() -> None:
108+
# A brand-new BaseException raised during teardown (not the one being unwound)
109+
# must still propagate rather than be silently swallowed.
110+
async def gen() -> AsyncGenerator[None, None]:
111+
try:
112+
yield
113+
finally:
114+
raise KeyboardInterrupt
115+
116+
g = gen()
117+
await g.__anext__()
118+
119+
with pytest.raises(KeyboardInterrupt):
120+
await async_clean_exit_stack([(g, True)])

wireup/ioc/_exit_stack.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,20 @@ def clean_exit_stack(
3333
except Exception as e: # noqa: BLE001
3434
if e is not exc_val:
3535
errors.append(e)
36+
except BaseException as e:
37+
# CancelledError / KeyboardInterrupt / SystemExit are BaseException, not
38+
# Exception. When the exception being unwound (exc_val) re-emerges from a
39+
# generator's teardown, keep closing the remaining generators instead of
40+
# letting it abort cleanup, which would skip exit_stack.clear() and leak
41+
# every earlier-registered resource. A genuinely new one is propagated.
42+
if e is not exc_val:
43+
raise
3644

3745
exit_stack.clear()
3846
maybe_raise_exc(exc_val=exc_val, exc_tb=exc_tb, container_close_errors=errors)
3947

4048

41-
async def async_clean_exit_stack(
49+
async def async_clean_exit_stack( # noqa: C901
4250
exit_stack: ExitStack,
4351
exc_val: BaseException | None = None,
4452
exc_tb: TracebackType | None = None,
@@ -64,6 +72,14 @@ async def async_clean_exit_stack(
6472
except Exception as e: # noqa: BLE001
6573
if e is not exc_val:
6674
errors.append(e)
75+
except BaseException as e:
76+
# CancelledError / KeyboardInterrupt / SystemExit are BaseException, not
77+
# Exception. When the exception being unwound (exc_val) re-emerges from a
78+
# generator's teardown, keep closing the remaining generators instead of
79+
# letting it abort cleanup, which would skip exit_stack.clear() and leak
80+
# every earlier-registered resource. A genuinely new one is propagated.
81+
if e is not exc_val:
82+
raise
6783

6884
exit_stack.clear()
6985
maybe_raise_exc(exc_val=exc_val, exc_tb=exc_tb, container_close_errors=errors)

0 commit comments

Comments
 (0)