From c0ff5e91696279ad2f1ea1c3c8c80f196da1e9ed Mon Sep 17 00:00:00 2001 From: Frowtek Date: Sat, 18 Jul 2026 16:46:59 +0300 Subject: [PATCH] fix(photon): run the Spectrum patch spawn off the gateway event loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PhotonAdapter._start_sidecar` is `async`, but it ran the Spectrum mixed-attachment patch script with a bare `subprocess.run(...)`: it spawns node and *waits* for it, with `timeout=10`. Executed inline that holds the shared gateway event loop for the whole window, so no other platform's messages, heartbeats, or sessions are serviced until it returns. The same function already establishes this exact invariant twenty lines above, where the stale-dependency reinstall hops to a worker thread: # Runs off the event loop so a cold install can't freeze every other # platform's traffic. if _sidecar_deps_stale(): await asyncio.to_thread(_reinstall_sidecar_deps) The patch spawn never got the same treatment. It is not startup-only either — `_start_sidecar` is called from `connect()`, which takes `is_reconnect`, so an ordinary Photon reconnect (network blip, sidecar death) re-runs it and stalls a live gateway that is actively serving Discord/Telegram/Slack traffic. Dispatch it via `asyncio.to_thread` like its sibling. Same off-the-loop class as the inbound-image decision (#66688) and the cron-fire verifier. Adds a regression test asserting the spawn executes on a worker thread rather than the loop thread. --- plugins/platforms/photon/adapter.py | 9 ++- .../photon/test_sidecar_lifecycle.py | 67 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/plugins/platforms/photon/adapter.py b/plugins/platforms/photon/adapter.py index 6ecd67bdde2f..4d49ccbdacfd 100644 --- a/plugins/platforms/photon/adapter.py +++ b/plugins/platforms/photon/adapter.py @@ -1199,7 +1199,14 @@ class PhotonAdapter(BasePlatformAdapter): from hermes_cli._subprocess_compat import windows_hide_flags try: - patch = subprocess.run( # noqa: S603 + # Off the event loop, for the same reason the dep reinstall above + # hops to a thread: this spawns node and *waits* for it (up to 10s). + # Run inline it holds the shared gateway loop for that whole window, + # so every other platform's traffic stalls — and _start_sidecar runs + # on every reconnect (connect(is_reconnect=True)), not just startup, + # so the stall recurs on a live gateway. + patch = await asyncio.to_thread( + subprocess.run, # noqa: S603 [ self._node_bin, str(_SIDECAR_DIR / "patch-spectrum-mixed-attachments.mjs"), diff --git a/tests/plugins/platforms/photon/test_sidecar_lifecycle.py b/tests/plugins/platforms/photon/test_sidecar_lifecycle.py index 5049736f9b57..717efddeb7a3 100644 --- a/tests/plugins/platforms/photon/test_sidecar_lifecycle.py +++ b/tests/plugins/platforms/photon/test_sidecar_lifecycle.py @@ -253,3 +253,70 @@ async def test_reap_inspects_listeners_off_the_event_loop( "must be dispatched via asyncio.to_thread so a 5s lsof/ps spawn " "can't freeze every other platform on the gateway loop" ) + + +@pytest.mark.asyncio +async def test_spectrum_patch_runs_off_the_event_loop( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The node patch run must not block the shared gateway event loop. + + ``_start_sidecar`` spawns the Spectrum patch script and *waits* for it + (``timeout=10``). Run inline it holds the loop for that whole window, so + every other platform's traffic stalls — and ``_start_sidecar`` runs on + every reconnect (``connect(is_reconnect=True)``), not just startup, so the + stall recurs on a live gateway. The dep reinstall a few lines above already + hops to a thread for exactly this reason; the patch run must too. + """ + import threading + + adapter = _make_adapter(monkeypatch) + main_thread = threading.current_thread() + seen: Dict[str, Any] = {} + + # node_modules present + deps fresh, so we reach the patch run. + monkeypatch.setattr(photon_adapter.Path, "exists", lambda self: True) + monkeypatch.setattr(photon_adapter, "_sidecar_deps_stale", lambda: False) + + async def _no_reap() -> None: + return None + + monkeypatch.setattr(adapter, "_reap_stale_sidecar", _no_reap) + + def _fake_run(*a: Any, **k: Any) -> Any: + seen["thread"] = threading.current_thread() + + class _Done: + returncode = 0 + stdout = "" + stderr = "" + + return _Done() + + monkeypatch.setattr(photon_adapter.subprocess, "run", _fake_run) + + class _FakeProc: + pid = 4242 + stdin = None + stdout = None + + def poll(self) -> None: + return None + + monkeypatch.setattr( + photon_adapter.subprocess, "Popen", lambda *a, **k: _FakeProc() + ) + + try: + await adapter._start_sidecar() + except Exception: + # Readiness/handshake past the patch run may fail under the fakes — + # irrelevant here; we only assert where the patch run executed. + pass + + assert seen.get("thread") is not None, "patch run never executed" + assert seen["thread"] is not main_thread, ( + "Spectrum patch subprocess ran on the event-loop thread; it must be " + "dispatched via asyncio.to_thread so a 10s node spawn can't freeze " + "every other platform on the gateway loop" + )