fix(serve): lock shared session state and throttle progress writes

Handlers share one service instance, so a per-session lock keeps two requests out of it at once. Track threads merge and write job state under one lock, and the progress sink coalesces ticks that say the same thing.

tests/remote/e2e/conftest.py

@@ -18,7 +18,7 @@ def load_scenarios(config: pytest.Config) -> dict:
     import yaml
 
     path = Path(__file__).parent / "fixtures" / "fixtures.yaml"
-    data = yaml.safe_load(path.read_text()) if path.exists() else {}
+    data = yaml.safe_load(path.read_text(encoding="utf-8")) if path.exists() else {}
     selected = (config.getoption("--services") or "").strip()
     services = (data or {}).get("services", {}) or {}
     if selected:

tests/remote/unit/test_handler_event_loop.py

@@ -0,0 +1,101 @@
+"""Blocking service calls must not run on the aiohttp event loop.
+
+get_titles and get_tracks do network work that can take a whole manifest round trip. Run on
+the loop, they stop the server: the progress pump and the SSE keep-alive stall with them. An
+interactive prompt from the service can never be answered either, because the endpoint that
+answers it needs the same loop. Each test here holds the service call in a two-event
+handshake that only completes while the loop still runs another coroutine.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import json
+import threading
+from types import SimpleNamespace
+from typing import Any, Awaitable, Callable, List
+
+import pytest
+from aiohttp import web
+
+from unshackle.core.api import handlers
+from unshackle.core.api.session_store import get_session_store
+from unshackle.core.titles.movie import Movie
+
+pytestmark = pytest.mark.unit
+
+# Bounds the wait when the loop is blocked, so a regression fails the test instead of hanging.
+HANDSHAKE_TIMEOUT = 5.0
+
+
+class Handshake:
+    """A blocking call that reports whether the event loop kept running during it."""
+
+    def __init__(self) -> None:
+        self.entered = threading.Event()
+        self.resume = threading.Event()
+        self.loop_was_blocked = False
+
+    def block(self) -> None:
+        self.entered.set()
+        self.loop_was_blocked = not self.resume.wait(timeout=HANDSHAKE_TIMEOUT)
+
+    async def drive(self, call: Callable[[], Awaitable[web.Response]]) -> web.Response:
+        task = asyncio.create_task(call())
+        # Reached only while the loop is free.
+        await asyncio.to_thread(self.entered.wait, HANDSHAKE_TIMEOUT)
+        self.resume.set()
+        return await asyncio.wait_for(task, HANDSHAKE_TIMEOUT)
+
+
+def a_movie() -> Movie:
+    return Movie(id_="movie-0001", service=object, name="Film", year=2024, language=None)
+
+
+class BlockingTitles:
+    def __init__(self, handshake: Handshake) -> None:
+        self.handshake = handshake
+
+    def get_titles(self) -> List[Movie]:
+        self.handshake.block()
+        return [a_movie()]
+
+
+class BlockingTracks:
+    def __init__(self, handshake: Handshake) -> None:
+        self.handshake = handshake
+
+    def get_titles(self) -> List[Movie]:
+        return [a_movie()]
+
+    def get_tracks(self, title: Any) -> Any:
+        self.handshake.block()
+        return SimpleNamespace(videos=[], audio=[], subtitles=[])
+
+
+@pytest.mark.asyncio
+async def test_session_titles_leaves_the_loop_free() -> None:
+    handshake = Handshake()
+    store = get_session_store()
+    await store.create("EXAMPLE", BlockingTitles(handshake), session_id="loop-test-titles")
+    try:
+        response = await handshake.drive(lambda: handlers.session_titles_handler("loop-test-titles"))
+    finally:
+        await store.delete("loop-test-titles")
+
+    assert not handshake.loop_was_blocked, "get_titles blocked the event loop"
+    assert [t["name"] for t in json.loads(response.body)["titles"]] == ["Film"]
+
+
+@pytest.mark.asyncio
+async def test_list_tracks_leaves_the_loop_free(monkeypatch: pytest.MonkeyPatch) -> None:
+    handshake = Handshake()
+    service = BlockingTracks(handshake)
+    monkeypatch.setattr(handlers, "validate_service", lambda tag, request=None: "EXAMPLE")
+    monkeypatch.setattr(handlers, "setup_list_service", lambda *args, **kwargs: service)
+
+    data = {"service": "EXAMPLE", "title_id": "movie-0001"}
+    response = await handshake.drive(lambda: handlers.list_tracks_handler(data))
+
+    assert not handshake.loop_was_blocked, "get_tracks blocked the event loop"
+    assert json.loads(response.body)["title"]["name"] == "Film"

tests/remote/unit/test_progress_throttle.py

@@ -0,0 +1,126 @@
+"""Unit tests for the progress write rate and for concurrent progress writers.
+
+The aggregate sink rewrites a whole JSON file per call, and one track sends thousands of ticks
+while the reader polls twice a second. These tests pin the two guards on that: repeat ticks are
+throttled, a tick that changes the job shape is not, and two threads writing the same file at the
+same time do not lose a write."""
+
+from __future__ import annotations
+
+import json
+import threading
+from pathlib import Path
+
+import pytest
+
+from unshackle.core.api import progress as progress_module
+from unshackle.core.api.download_worker import write_result
+from unshackle.core.api.progress import build_job_progress_callables
+
+pytestmark = pytest.mark.unit
+
+
+class Video:
+    def __init__(self, height=1080, bitrate=4_000_000):
+        self.height = height
+        self.range = None
+        self.bitrate = bitrate
+
+
+def noop(**kwargs):
+    pass
+
+
+def test_repeat_ticks_are_throttled():
+    updates: list[dict] = []
+    cbs = build_job_progress_callables([Video()], [noop], updates.append)
+
+    for completed in range(1, 501):
+        cbs[0](total=1000, completed=completed)
+
+    # One emit for the first tick (the track starts); the rest say the same thing and coalesce.
+    assert len(updates) == 1
+
+
+def test_track_finish_bypasses_the_throttle():
+    updates: list[dict] = []
+    cbs = build_job_progress_callables([Video(), Video()], [noop, noop], updates.append)
+
+    cbs[0](total=100, completed=1)
+    cbs[0](total=100, completed=2)
+    cbs[0](total=100, completed=3)
+    assert len(updates) == 1
+
+    cbs[0](downloaded="Downloaded")
+    assert len(updates) == 2
+    assert updates[-1]["completed_tracks"] == 1
+
+
+def test_phase_change_bypasses_the_throttle():
+    updates: list[dict] = []
+    cbs = build_job_progress_callables([Video(2160), Video(720)], [noop, noop], updates.append)
+
+    cbs[0](total=100, completed=1)
+    first_phase = updates[-1]["phase"]
+
+    cbs[1](total=100, completed=1)
+    assert len(updates) == 2
+    assert updates[-1]["phase"] != first_phase
+
+
+def test_filled_track_bypasses_the_throttle_without_a_terminal_string():
+    """A track can reach its segment total and never send "Downloaded". That last tick must land,
+    or the job reports a percentage that is short for good."""
+    updates: list[dict] = []
+    cbs = build_job_progress_callables([Video()], [noop], updates.append)
+
+    cbs[0](total=100, completed=10)
+    cbs[0](total=100, completed=50)
+    assert len(updates) == 1
+
+    cbs[0](total=100, completed=100)
+    assert len(updates) == 2
+    assert updates[-1]["progress"] == pytest.approx(progress_module.DOWNLOAD_PROGRESS_CEILING)
+
+
+def test_throttle_releases_after_the_interval(monkeypatch: pytest.MonkeyPatch):
+    clock = {"now": 1000.0}
+    monkeypatch.setattr(progress_module.time, "monotonic", lambda: clock["now"])
+
+    updates: list[dict] = []
+    cbs = build_job_progress_callables([Video()], [noop], updates.append)
+
+    cbs[0](total=100, completed=1)
+    cbs[0](total=100, completed=2)
+    assert len(updates) == 1
+
+    clock["now"] += progress_module.PROGRESS_EMIT_INTERVAL + 0.01
+    cbs[0](total=100, completed=3)
+    assert len(updates) == 2
+
+
+def test_concurrent_writers_do_not_clobber_each_other(tmp_path: Path):
+    """Two threads can write the same progress file. A temp name shared by two writers lets one
+    replace away the file the other is about to move, which fails that write."""
+    target = tmp_path / "progress.json"
+    errors: list[BaseException] = []
+    start = threading.Barrier(8)
+
+    def writer(index: int) -> None:
+        start.wait()
+        try:
+            for tick in range(40):
+                write_result(target, {"writer": index, "tick": tick})
+        except BaseException as exc:  # noqa: BLE001 - reported to the test thread
+            errors.append(exc)
+
+    threads = [threading.Thread(target=writer, args=(i,)) for i in range(8)]
+    for thread in threads:
+        thread.start()
+    for thread in threads:
+        thread.join()
+
+    assert errors == []
+    payload = json.loads(target.read_text(encoding="utf-8"))
+    assert set(payload) == {"writer", "tick"}
+    assert [p.name for p in tmp_path.iterdir()] == ["progress.json"]