From 2fa04337cd9dd6f738e2d19e78396a2fd2611eef Mon Sep 17 00:00:00 2001 From: zigpy-review-bot <286747149+zigpy-review-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:44:32 +0200 Subject: [PATCH] Clear `EventLoopThread.loop` before closing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_thread_main`'s `finally` closed the loop and only then set `self.loop = None`, so between those two statements `self.loop` named an already-closed loop — the window #727 had to defend against with an `is_closed()` guard plus a suppressed `RuntimeError` in `force_stop`. Publishing `None` first makes the invariant "`self.loop` is either `None` or an open loop" hold for every reader, and it also means the exiting thread never writes `self.loop` again after that point — previously the trailing `self.loop = None` could clobber the loop a concurrent `start()` had just installed. #727's `suppress(RuntimeError)` in `force_stop` stays: a caller can still snapshot the loop just before `close()` runs. --- bellows/thread.py | 7 +++++-- tests/test_thread.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/bellows/thread.py b/bellows/thread.py index d35f1d34..a10b58d9 100644 --- a/bellows/thread.py +++ b/bellows/thread.py @@ -27,8 +27,11 @@ def _thread_main(self, init_task): self.loop.run_until_complete(init_task) self.loop.run_forever() finally: - self.loop.close() - self.loop = None + # Publish `None` before closing: `self.loop` then never names a closed loop, and + # this thread no longer writes `self.loop` after a concurrent `start()` may have + # replaced it. + loop, self.loop = self.loop, None + loop.close() async def start(self): current_loop = asyncio.get_event_loop() diff --git a/tests/test_thread.py b/tests/test_thread.py index 72efa701..235507f7 100644 --- a/tests/test_thread.py +++ b/tests/test_thread.py @@ -29,6 +29,37 @@ def mockrun(task): assert loopmock.close.call_count == 1 +async def test_thread_clears_loop_before_closing(monkeypatch): + """`self.loop` is cleared before the loop is closed, so it never names a closed loop.""" + thread = EventLoopThread() + real_new_event_loop = asyncio.new_event_loop + loop_at_close = mock.sentinel.close_not_called + + def new_event_loop(): + loop = real_new_event_loop() + real_close = loop.close + + def close(): + nonlocal loop_at_close + loop_at_close = thread.loop + real_close() + + loop.close = close + return loop + + monkeypatch.setattr(asyncio, "new_event_loop", new_event_loop) + + thread_complete = await thread.start() + thread.force_stop() + async with asyncio_timeout(1): + await thread_complete + + assert loop_at_close is None + assert thread.loop is None + + [t.join(1) for t in threading.enumerate() if "bellows" in t.name] + + class ExceptionCollector: def __init__(self): self.exceptions = []