From 803ab170d84d858d3ff741d869ff38d8fac5712b Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Fri, 15 May 2026 17:12:29 +0000 Subject: [PATCH] stage-360: align test_sprint29::TestENVLock with non-reentrant invariant from QA #2299 added test_env_lock_importable_from_streaming asserting reentrance, which contradicts the architectural invariant enforced by QA test_env_lock_is_non_reentrant. The QA test wins because the non-reentrant property is what makes _ENV_LOCK catch deadlock bugs early. Updated the new test to assert NON-reentrance to match the actual lock type (threading.Lock) and the QA invariant. --- tests/test_sprint29.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/test_sprint29.py b/tests/test_sprint29.py index 4050666e..9a6e0da1 100644 --- a/tests/test_sprint29.py +++ b/tests/test_sprint29.py @@ -712,7 +712,15 @@ class TestSSRFCheck: class TestENVLock: def test_env_lock_importable_from_streaming(self): - """_ENV_LOCK must be an importable threading-style reentrant lock.""" + """_ENV_LOCK must be an importable threading-style lock. + + Stage-360 maintainer fix: _ENV_LOCK MUST be non-reentrant + (threading.Lock, not RLock) — see QA test_env_lock_is_non_reentrant. + RLock would mask the deadlock pattern that the lock is designed to + catch. Background workers that need profile env should use the + narrow-lock pattern in profile_env_for_background_worker() rather + than relying on reentrance. + """ from api.streaming import _ENV_LOCK assert hasattr(_ENV_LOCK, "acquire"), "_ENV_LOCK must expose acquire()" @@ -720,10 +728,15 @@ class TestENVLock: assert hasattr(_ENV_LOCK, "__enter__"), "_ENV_LOCK must support context manager use" assert hasattr(_ENV_LOCK, "__exit__"), "_ENV_LOCK must support context manager use" + # Verify non-reentrance: a same-thread second acquire(blocking=False) + # while the lock is held must fail. This invariant matters because + # it catches a class of deadlock bugs early. with _ENV_LOCK: - acquired = _ENV_LOCK.acquire(False) - assert acquired, "_ENV_LOCK must allow reentrant acquisition" - _ENV_LOCK.release() + acquired_again = _ENV_LOCK.acquire(False) + assert not acquired_again, ( + "_ENV_LOCK must be non-reentrant (threading.Lock, not RLock). " + "See QA test_env_lock_is_non_reentrant for the architectural reason." + ) def test_env_lock_importable_in_routes(self): """api.routes must be able to import _ENV_LOCK from api.streaming."""