diff --git a/backend/main.py b/backend/main.py index d4baeafc..864ffc4d 100644 --- a/backend/main.py +++ b/backend/main.py @@ -688,8 +688,20 @@ async def lifespan(app: FastAPI): # (still importing, not yet mid weight-download) finish cleanly before we # report done; a load that's genuinely deep into a multi-GB download still # times out — _reset_gpu_pool() below abandons it either way. + # + # 20s, not the original 3s (code-review finding post-merge): a cold + # transformers import alone can take longer than 3s on a slow disk or a + # first-ever launch, so the original bound left a real residual window — + # cancellation detaches the asyncio task, but the underlying OS thread + # keeps running past it, and shutdown could still report "done" while + # that thread was alive. Python cannot forcibly kill a running thread, so + # no finite bound eliminates this outright — 20s just shrinks the window + # from "any preload" to "an unusually slow cold-import," which is the + # practical ceiling before a longer shutdown itself becomes the + # complaint. A thread that's still running past 20s was never going to + # finish in a shutdown-appropriate timeframe regardless. await _cancel_and_await_tasks( - idle_task, worker_task, preload_task, capture_preload_task, timeout=3.0, + idle_task, worker_task, preload_task, capture_preload_task, timeout=20.0, ) # Unload the model and free GPU memory try: diff --git a/tests/test_engines.py b/tests/test_engines.py index c0ecdc72..16e962d6 100644 --- a/tests/test_engines.py +++ b/tests/test_engines.py @@ -400,6 +400,27 @@ def test_mlx_audio_generate_omits_ref_text_without_ref_audio(): assert "ref_text" not in captured +def test_mlx_audio_generate_design_path_unaffected_without_any_ref(): + # Absorbed from community PR #1015 (MahdiHedhli) — the design/instruct + # path (no ref_audio, no ref_text at all) must stay untouched by the + # ref_text forwarding fix; neither kwarg may leak into the model call. + pytest.importorskip("mlx_audio", reason="mlx-audio is Apple-Silicon-only") + backend = tts_backend.MLXAudioBackend() + backend._ensure_loaded = lambda: None + + captured = {} + + def _fake_generate(**kw): + captured.update(kw) + return iter([types.SimpleNamespace(audio=__import__("numpy").zeros(4))]) + + backend._model = types.SimpleNamespace(generate=_fake_generate) + backend.generate("hello") + + assert "ref_text" not in captured + assert "ref_audio" not in captured + + def test_mlx_audio_generate_auto_language_skips_lang_code_entirely(): # Matches the "Auto" convention other engines in this file use # (OmniVoiceBackend.generate(), _run_backend_inference) — never resolved, diff --git a/tests/test_shutdown_preload_race_1000.py b/tests/test_shutdown_preload_race_1000.py index ab3e2d05..301afa9d 100644 --- a/tests/test_shutdown_preload_race_1000.py +++ b/tests/test_shutdown_preload_race_1000.py @@ -110,3 +110,34 @@ def test_multiple_tasks_are_all_cancelled_before_any_await(): _run(_scenario()) assert set(cancelled_order) == {"slow", "fast"} + + +def test_production_shutdown_wait_is_generous_enough_for_a_cold_import(): + """Post-merge code-review finding (Greptile, PR #1002): the original 3s + bound left a real residual window — cancelling the asyncio task doesn't + stop the underlying OS thread, so a cold transformers import taking + longer than the bound could still let shutdown report "done" while that + thread was alive, the exact #1000 class again just with lower odds. + Python can't forcibly kill a running thread, so no finite bound + eliminates this outright — this pins the production call site to a + materially more generous wait (20s, not 3s) rather than letting a future + edit quietly shrink it back down without deliberate consideration. + + Source-level guard, not a live-timing test: driving an actual >3s cold + import through this suite would make it slow and environment-dependent + for no real benefit. + """ + import re + + src = open(os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + "backend", "main.py")).read() + call = re.search( + r"await _cancel_and_await_tasks\(\s*idle_task,\s*worker_task,\s*preload_task," + r"\s*capture_preload_task,\s*timeout=([\d.]+),?\s*\)", + src, + ) + assert call, "production shutdown call site not found in main.py" + assert float(call.group(1)) >= 15.0, ( + f"shutdown wait bound regressed to {call.group(1)}s — see PR #1002 review history " + "before shrinking this" + )