From 13c14e2cc03680e34ddf75f413f814c93ea2658f Mon Sep 17 00:00:00 2001 From: Paolo Antinori Date: Thu, 20 Aug 2026 15:47:27 +0200 Subject: [PATCH] fix(openai-compat): unload the outgoing engine on explicit-ID switches Review follow-up (Greptile/CodeRabbit on #1614): caching instances without a switch rule would let each distinct explicit engine ID stay resident, accumulating sidecars / multi-GB in-process models. Mirror get_active_tts_backend's MM2-01 switch rule: a different explicit ID (omnivoice included, which resolves to the active engine) unloads the outgoing instance first, best-effort. --- backend/api/routers/openai_compat.py | 30 +++++++++++++++++++- tests/test_openai_speech_engine_cache.py | 36 ++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/backend/api/routers/openai_compat.py b/backend/api/routers/openai_compat.py index 30d3fb45..f32e7ad0 100644 --- a/backend/api/routers/openai_compat.py +++ b/backend/api/routers/openai_compat.py @@ -158,6 +158,25 @@ _OPENAI_VOICE_ALIASES = { # ── TTS: POST /v1/audio/speech ────────────────────────────────────────────── +#: Last engine explicitly requested via a `model` ID on this route, and its +#: instance — mirrors get_active_tts_backend's switch rule (MM2-01): loading a +#: different explicit engine unloads the outgoing one first, so cached explicit +#: IDs can't accumulate multi-GB in-process models / sidecars. +_explicit_engine: dict = {} + + +def _unload_explicit_engine() -> None: + inst = _explicit_engine.get("instance") + if inst is None: + return + try: + inst.unload() + except Exception as exc: # noqa: BLE001 — a bad unload must not block a switch + logger.warning("explicit engine switch: %s.unload() raised: %s", + type(inst).__name__, exc) + _explicit_engine.clear() + + def _resolve_engine(model_id: str): """Map an OpenAI model name to a VoiceStudio backend.""" from services.tts_backend import ( @@ -179,11 +198,20 @@ def _resolve_engine(model_id: str): ) from services.tts_backend import OmniVoiceBackend if cls is OmniVoiceBackend: + # OmniVoice only ever runs as the shared active engine — the + # explicit-omnivoice request is the active-engine request. + _unload_explicit_engine() return get_active_tts_backend() # Cached singleton, not a fresh cls(): SubprocessBackend engines would # spawn a sidecar process and reload their model on EVERY request, and # register a new atexit hook each time (get_engine_instance's contract). - return get_engine_instance_for(model_id) + # And on a switch between explicit IDs, unload the outgoing engine so + # the cache can't accumulate residents (same rule as the active path). + if _explicit_engine.get("id") != model_id: + _unload_explicit_engine() + _explicit_engine["id"] = model_id + _explicit_engine["instance"] = get_engine_instance_for(model_id) + return _explicit_engine["instance"] except ValueError: raise HTTPException( status_code=400, diff --git a/tests/test_openai_speech_engine_cache.py b/tests/test_openai_speech_engine_cache.py index c1224c09..69cee325 100644 --- a/tests/test_openai_speech_engine_cache.py +++ b/tests/test_openai_speech_engine_cache.py @@ -62,3 +62,39 @@ def test_unknown_engine_id_still_400s(oc, monkeypatch): oc._resolve_engine("not-an-engine") assert exc.value.status_code == 400 assert "Unknown model" in exc.value.detail + + +def test_switching_explicit_engine_ids_unloads_the_outgoing_one(oc, monkeypatch): + """Cache must not accumulate residents: a different explicit `model` ID + unloads the outgoing engine first (same switch rule as the active-engine + path, MM2-01).""" + svc = _tts_mod() + unloaded = [] + + def _make(name): + class _B: + ident = name + + def __init__(self): + pass + + @staticmethod + def is_available(): + return True, "ok" + + def unload(self): + unloaded.append(name) + + return _B + + engines = {"a-engine": _make("a"), "b-engine": _make("b")} + monkeypatch.setattr(svc, "get_backend_class", lambda i: engines[i]) + + first = oc._resolve_engine("a-engine") + second = oc._resolve_engine("a-engine") + assert first is second + assert unloaded == [] + + third = oc._resolve_engine("b-engine") + assert third is not first + assert unloaded == ["a"]