From 551ad64c66f4f4f0f7a98edc674fbcdf78e98a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Fri, 28 Aug 2026 11:26:16 +0200 Subject: [PATCH] fix: widen thread-leak fixture's grace period to match stop()'s own timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ElectronicControlUnit.stop() joins the dispatch thread with a default dispatch_join_timeout of 3.0s, and only logs a warning (rather than failing) if the thread is still alive after that — tolerating slow subscriber callbacks by design. The autouse _assert_no_j1939_thread_leak fixture in conftest.py was only polling for 200ms, well under stop()'s own tolerance, which could produce false-positive leak failures for any test exercising a slow-callback shutdown path under scheduling load. Widen the fixture's poll window to 3.5s so it never gives up sooner than stop() itself considers acceptable, without weakening the check for tests that genuinely leak (the loop still exits early once the leaked threads disappear). Fixes #81 Co-Authored-By: Claude Sonnet 5 --- test/conftest.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index e3f14bf..6afbf9b 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -21,18 +21,28 @@ def feeder(): @pytest.fixture(autouse=True) def _assert_no_j1939_thread_leak(): - """Fail any test that leaves a j1939.* background thread alive.""" + """Fail any test that leaves a j1939.* background thread alive. + + The poll window here (3.5s) is deliberately kept slightly above + ElectronicControlUnit.stop()'s own default dispatch_join_timeout (3.0s): + stop() only logs a warning (rather than failing) if the dispatch thread + doesn't exit within that timeout, e.g. while a slow subscriber callback + unwinds. If this fixture's window were shorter than stop()'s own + tolerance, a thread that stop() itself considers "still fine, just slow" + could trip a false-positive leak failure here. See #81. + """ before = {t.ident for t in threading.enumerate() if t.name.startswith('j1939.')} yield - # Give freshly-stopped threads a brief moment to actually exit. + # Give freshly-stopped threads a chance to actually exit. import time - for _ in range(20): + deadline = time.monotonic() + 3.5 + while True: leaked = [t for t in threading.enumerate() if t.name.startswith('j1939.') and t.ident not in before and t.is_alive()] - if not leaked: + if not leaked or time.monotonic() >= deadline: break time.sleep(0.01) assert not leaked, (