2477d992f4
* fix(streaming): lock-disciplined STREAMS reads via peek_stream() Writers pop STREAMS under STREAMS_LOCK, but four hot reads used bare STREAMS.get(): the streaming worker start, the gateway run start, and the two SSE attach paths in routes.py. Each is individually saved today by GIL-atomic dict.get plus a None-guard, but the check-then-act windows (get → register, get → subscribe) race teardown pops, and the bare pattern invites unprotected copies. peek_stream() centralizes the locked read; all four sites migrate with no behavioral change. * test: dual-patch config.STREAMS alongside routes.STREAMS in SSE seam tests peek_stream() reads the canonical api.config.STREAMS registry; tests that inject a fake registry by patching only the routes-level re-export left the helper reading the real (empty) registry and hung the SSE loop to the pytest-timeout ceiling. Mirror the test_issue6623 pattern: patch both bindings. * docs(changelog): stamp #7339 lock-disciplined STREAMS reads --------- Co-authored-by: Sebastian B <sb@wmc.sh> Co-authored-by: n <a@n>
72 lines
2.3 KiB
Python
72 lines
2.3 KiB
Python
"""STREAMS registry reads must go through the lock-disciplined peek_stream().
|
|
|
|
Writers mutate config.STREAMS under STREAMS_LOCK (teardown in api/streaming.py
|
|
and the route layer); bare STREAMS.get() reads race those pops. These tests pin
|
|
the peek_stream() helper and forbid reintroducing bare STREAMS.get() calls in
|
|
the api package outside api/config.py itself.
|
|
"""
|
|
|
|
import queue
|
|
import re
|
|
import threading
|
|
from pathlib import Path
|
|
|
|
import api.config as config
|
|
|
|
REPO = Path(__file__).resolve().parents[1]
|
|
API_DIR = REPO / "api"
|
|
|
|
|
|
def test_peek_stream_returns_registered_queue_and_none_for_missing():
|
|
stream_id = "peek-stream-registered"
|
|
q = queue.Queue()
|
|
registered = False
|
|
try:
|
|
with config.STREAMS_LOCK:
|
|
config.STREAMS[stream_id] = q
|
|
registered = True
|
|
assert config.peek_stream(stream_id) is q
|
|
assert config.peek_stream("nope") is None
|
|
finally:
|
|
if registered:
|
|
with config.STREAMS_LOCK:
|
|
config.STREAMS.pop(stream_id, None)
|
|
|
|
|
|
def test_peek_stream_acquires_streams_lock(monkeypatch):
|
|
events = []
|
|
real_lock = threading.Lock()
|
|
|
|
class _RecordingLock:
|
|
def __enter__(self):
|
|
events.append("enter")
|
|
real_lock.acquire()
|
|
return real_lock
|
|
|
|
def __exit__(self, exc_type, exc, tb):
|
|
real_lock.release()
|
|
return False
|
|
|
|
monkeypatch.setattr(config, "STREAMS_LOCK", _RecordingLock())
|
|
# Missing id: the locked lookup still happens and returns None.
|
|
assert config.peek_stream("peek-stream-lock-check") is None
|
|
assert events, "peek_stream did not acquire STREAMS_LOCK"
|
|
|
|
|
|
def test_no_bare_streams_get_outside_config():
|
|
pattern = re.compile(r"STREAMS\.get\(")
|
|
offenders = []
|
|
for path in sorted(API_DIR.glob("*.py")):
|
|
if path.name == "config.py":
|
|
continue
|
|
text = path.read_text(encoding="utf-8")
|
|
for lineno, line in enumerate(text.splitlines(), start=1):
|
|
if pattern.search(line):
|
|
offenders.append(
|
|
f"{path.relative_to(REPO)}:{lineno}: {line.strip()}"
|
|
)
|
|
assert not offenders, (
|
|
"bare STREAMS.get( outside api/config.py; use config.peek_stream() "
|
|
"so reads take STREAMS_LOCK:\n" + "\n".join(offenders)
|
|
)
|