test: address Copilot review on the leaked-thread guard

- 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.
This commit is contained in:
Patrick Buckley
2026-06-17 18:05:27 -07:00
parent a7d8895287
commit 73f4fb5933
2 changed files with 7 additions and 3 deletions
+5 -2
View File
@@ -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():
+2 -1
View File
@@ -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