Skip to content

Commit c0337f7

Browse files
committed
Fix: pytest never exits after the summary, wedged by abandoned non-daemon threads
Every CI job printed its pytest summary and then sat there until the fifteen-minute cap killed it -- 5m41s of dead time on ubuntu/3.11, and the same on all eight matrix jobs. Root cause: tests/comprehensive/test_edge_cases_real.py::test_deadlock_prevention builds a deliberate AB-BA lock inversion in two *non-daemon* threads, joins them with timeout=5, and abandons them when (as designed) they deadlock. Py_FinalizeEx calls threading._shutdown(), which joins every surviving non-daemon thread with no timeout, so the interpreter could never finalize. The test is also re-run by reflection from test_comprehensive_edge_case_suite, so four threads were wedged, not two. Evidence, from a faulthandler dump armed at pytest_unconfigure: AT-UNCONFIGURE non-daemon alive: 4 ALIVE Thread-6 target=...potential_deadlock.<locals>.worker1 ALIVE Thread-7 target=...potential_deadlock.<locals>.worker2 ALIVE Thread-8 target=...potential_deadlock.<locals>.worker1 ALIVE Thread-9 target=...potential_deadlock.<locals>.worker2 Thread 0x33da2b000: test_edge_cases_real.py line 635 in worker2 Thread 0x33ca1f000: test_edge_cases_real.py line 629 in worker1 Thread 0x33ba13000: test_edge_cases_real.py line 635 in worker2 Thread 0x33aa07000: test_edge_cases_real.py line 629 in worker1 Thread 0x1fbd91d80: threading.py line 1477 in _shutdown Marking those two threads daemon changes nothing the test observes -- the deadlock still happens, the joins still time out, is_alive() is still True and the assertions are untouched -- but abandoning them no longer wedges the process. Same defect, second site: tests/real_world/conftest.py::_within claimed to abandon its worker, but a ThreadPoolExecutor's workers are non-daemon and concurrent.futures joins all of them, untimed, at interpreter exit even after shutdown(wait=False). Abandoning a seventy-second DNS lookup only moved the wait to process exit. Replaced with an actual daemon thread, preserving the existing semantics (OSError -> None, timeout -> None, anything else re-raised). Isolating measurement, tests/comprehensive/test_edge_cases_real.py alone: before: summary at 59.98s, still not exited 90s later after: summary at 62.60s, exited 0.66s later Full CI selection: before: [345.05] 1764 passed ... -- never exited, killed after 6 minutes after: [343.58] 1764 passed ... / [345.64] EXITED rc=0
1 parent 1af2c02 commit c0337f7

2 files changed

Lines changed: 40 additions & 15 deletions

File tree

tests/comprehensive/test_edge_cases_real.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -635,9 +635,19 @@ def worker2():
635635
with lock1:
636636
results.append("worker2")
637637

638-
# Use timeout to prevent actual deadlock
639-
t1 = threading.Thread(target=worker1)
640-
t2 = threading.Thread(target=worker2)
638+
# These two workers take their locks in opposite orders, so the
639+
# deadlock below is the point of the test, not an accident: the
640+
# join timeout is what keeps *this* function from hanging.
641+
#
642+
# They must be daemons. The joins below give up after five
643+
# seconds and abandon threads that are wedged forever, and
644+
# `threading._shutdown()` joins every surviving non-daemon thread
645+
# with no timeout before the interpreter can finalize. Leaving
646+
# these non-daemon wedged the whole pytest process after the
647+
# summary line was printed -- every CI job burned its remaining
648+
# budget there and was killed at the fifteen-minute cap.
649+
t1 = threading.Thread(target=worker1, daemon=True)
650+
t2 = threading.Thread(target=worker2, daemon=True)
641651

642652
t1.start()
643653
t2.start()

tests/real_world/conftest.py

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
import pytest
77
from pathlib import Path
88
import tempfile
9-
import concurrent.futures
109
import functools
1110
import socket
11+
import threading
1212

1313
from tests.real_world import RealWorldTestManager, TestCredentials, TempResourceManager
1414

@@ -32,17 +32,32 @@ def _within(seconds, func, *args):
3232
The resolver calls here are not interruptible, so the worker thread is left
3333
to finish on its own; it is a daemon and holds nothing the caller needs.
3434
"""
35-
# Deliberately not a `with` block: its __exit__ calls shutdown(wait=True)
36-
# and blocks until the worker finishes, which defeats the timeout entirely
37-
# -- a call that should have been abandoned after three seconds still took
38-
# thirty.
39-
pool = concurrent.futures.ThreadPoolExecutor(max_workers=1)
40-
try:
41-
return pool.submit(func, *args).result(timeout=seconds)
42-
except (concurrent.futures.TimeoutError, OSError):
43-
return None
44-
finally:
45-
pool.shutdown(wait=False)
35+
# A plain daemon thread, not a ThreadPoolExecutor. The executor's workers
36+
# are non-daemon, and `concurrent.futures` joins every one of them --
37+
# untimed -- on the way out of the interpreter, even after
38+
# `shutdown(wait=False)`. Abandoning a seventy-second lookup that way only
39+
# moved the wait from here to process exit. A daemon thread is genuinely
40+
# abandonable: nothing joins it and the interpreter does not wait for it.
41+
outcome = {}
42+
43+
def call():
44+
try:
45+
outcome["value"] = func(*args)
46+
except BaseException as exc: # re-raised below, in the caller's thread
47+
outcome["error"] = exc
48+
49+
worker = threading.Thread(target=call, daemon=True)
50+
worker.start()
51+
worker.join(seconds)
52+
53+
error = outcome.get("error")
54+
if error is not None:
55+
# Unresolvable names are the expected off-network answer, not a fault.
56+
if isinstance(error, OSError):
57+
return None
58+
raise error
59+
# Absent on timeout, because the worker never got as far as storing one.
60+
return outcome.get("value")
4661

4762

4863
@functools.lru_cache(maxsize=1)

0 commit comments

Comments
 (0)