From 92c33896b5e8eab3101e5564e42eea40604df8df Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 26 Jul 2026 05:39:45 +0000 Subject: [PATCH] fix(python): stop teardown test flaking on unrelated thread reaps test_threads_joined_deterministically_after_drop asserted an exact process-wide thread count and failed intermittently in the shrink direction (assert 13 == 14 here, assert 15 == 16 on main at 082447e3), blocking unrelated PRs. Not a leak. tokio reaps idle blocking-pool threads on a 10 s timer, so a pytest session that ran async work earlier sheds an unrelated thread part-way through the churn loop. Measured: the count holds at 6 after an async workload and drops to 5 at exactly t=10.0 s. TM-PY-030 is about what a drop leaves behind, so the invariant is one-directional. Replaces the equality check with _assert_no_thread_growth(), matching what the sibling fd-churn test in the same file already did. A real leak compounds across the five iterations and still trips it. Adds two tests pinning the direction: an inflated baseline must pass, and growth must fail. Both fail against the old == form. --- .../tests/test_teardown_determinism.py | 35 +++++++++++++++++-- knowledge/runtimes/python-package.md | 8 +++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/bashkit-python/tests/test_teardown_determinism.py b/crates/bashkit-python/tests/test_teardown_determinism.py index 1aff8f011..36df3c243 100644 --- a/crates/bashkit-python/tests/test_teardown_determinism.py +++ b/crates/bashkit-python/tests/test_teardown_determinism.py @@ -42,6 +42,36 @@ def _make_tool() -> ScriptedTool: return t +def _assert_no_thread_growth(baseline: int) -> None: + """Assert teardown left nothing behind, relative to `baseline`. + + One-directional by design. TM-PY-030 is about what a drop *leaves behind*, + and the process-wide count is not this tool's to own: tokio reaps idle + blocking-pool threads on a 10 s timer, so a pytest session that ran async + work before this module sheds an unrelated thread part-way through the + churn loop. Measured directly — after an async workload the count holds at + 6 and drops to 5 at exactly t=10.0 s. Asserting exact equality read that + as a failure (`assert 13 == 14`) though nothing leaked; the count moved + *down*. A real leak still fails: it compounds across iterations and pushes + the count above the baseline. + """ + count = _native_thread_count() + assert count <= baseline, f"thread count grew: {baseline} -> {count}" + + +@pytest.mark.skipif(not PROC_AVAILABLE, reason="requires /proc") +def test_thread_check_tolerates_shrink_from_unrelated_pools(): + """An inflated baseline (a thread reaped after sampling) is not a defect.""" + _assert_no_thread_growth(_native_thread_count() + 1) + + +@pytest.mark.skipif(not PROC_AVAILABLE, reason="requires /proc") +def test_thread_check_rejects_growth(): + """The check still catches a leak — the direction that means a defect.""" + with pytest.raises(AssertionError, match="thread count grew"): + _assert_no_thread_growth(_native_thread_count() - 1) + + @pytest.mark.skipif(not PROC_AVAILABLE, reason="requires /proc") def test_threads_joined_deterministically_after_drop(): """del tool returns only after worker + runtime threads are joined.""" @@ -57,8 +87,9 @@ def test_threads_joined_deterministically_after_drop(): assert t.execute_sync("hit").exit_code == 0 del t gc.collect() - # Exact equality, immediately: joins are synchronous in drop. - assert _native_thread_count() == baseline + # Immediately: joins are synchronous in drop, so a tool's threads are + # gone before `del` returns and repeated churn cannot accumulate. + _assert_no_thread_growth(baseline) @pytest.mark.skipif(not PROC_AVAILABLE, reason="requires /proc") diff --git a/knowledge/runtimes/python-package.md b/knowledge/runtimes/python-package.md index e2f435d63..d056fdeec 100644 --- a/knowledge/runtimes/python-package.md +++ b/knowledge/runtimes/python-package.md @@ -203,6 +203,14 @@ handle is dropped *inside* a tokio context (a `Bash` dropped while blocking runtime join there would panic, so the drop falls back to `shutdown_background()` instead of the deterministic join. +The regression test asserts **no thread growth**, not an exact process-wide +thread count. tokio reaps idle blocking-pool threads on a 10 s timer, so a test +session that ran async work earlier sheds an unrelated thread mid-loop +(measured: 6 threads holding, dropping to 5 at exactly t=10.0 s). Exact +equality read that shrink as a failure though nothing had leaked. Do not +re-tighten it — the process-wide count is not a single tool's to own, and a +genuine leak compounds across the churn iterations and still trips the check. + **ContextVar propagation**: ContextVars set before `execute()` / `execute_sync()` are captured at call time and replayed inside each callback invocation regardless of mechanism.