From 0ae0db55f32ee89b47838df24766976c30c590b0 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Wed, 17 Jun 2026 18:05:27 -0700 Subject: [PATCH] test: address Copilot review on the leaked-thread guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The guard snapshotted live threads by `Thread.ident`, but idents are recycled after a thread exits — a new leaked thread reusing an exited thread's ident would be mistaken for pre-existing and missed (false negative). Snapshot the Thread OBJECTS and compare by identity instead. - Fix the `serve` fixture docstring: the factory returns the ephemeral port, not the server. --- tests/conftest.py | 7 +++++-- tests/test_docker_healthcheck.py | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5bfcf60f..cc1519ef 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -87,7 +87,10 @@ def _no_leaked_threads(request: pytest.FixtureRequest) -> Iterator[None]: if request.node.get_closest_marker("allow_thread_leak"): yield return - before = {t.ident for t in threading.enumerate()} + # Snapshot the Thread OBJECTS, not their idents: Thread.ident is recycled + # after a thread exits, so an ident-based snapshot could mistake a new + # leaked thread (reusing an exited thread's ident) for a pre-existing one. + before = set(threading.enumerate()) yield main = threading.main_thread() current = threading.current_thread() @@ -97,7 +100,7 @@ def _no_leaked_threads(request: pytest.FixtureRequest) -> Iterator[None]: deadline = time.monotonic() + _THREAD_LEAK_GRACE leaked = [] for t in threading.enumerate(): - if t.ident in before or t is main or t is current or not t.is_alive(): + if t in before or t is main or t is current or not t.is_alive(): continue t.join(timeout=max(0.0, deadline - time.monotonic())) if t.is_alive(): diff --git a/tests/test_docker_healthcheck.py b/tests/test_docker_healthcheck.py index cce73fc9..9c41f4b0 100644 --- a/tests/test_docker_healthcheck.py +++ b/tests/test_docker_healthcheck.py @@ -54,7 +54,8 @@ class _Handler(http.server.BaseHTTPRequestHandler): @pytest.fixture def serve(): - """Factory that starts an HTTP(S) server on an ephemeral port and returns it. + """Factory that starts an HTTP(S) server on an ephemeral port and returns + that port. Every server it starts is shut down + its serve_forever thread joined at teardown, so the thread never outlives the test (which would otherwise bleed