mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-31 14:10:20 +00:00
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.
This commit is contained in:
+17
-4
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user