fix(model): first-run model load/download never hangs forever (Windows demo-voice hang) - #173
Conversation
…ever Windows users reported 'create demo voice runs indefinitely, no audio, no error'. Root cause: the first /generate triggers OmniVoice.from_pretrained() which downloads multi-GB weights via the legacy LFS path (HF_HUB_DISABLE_XET=1), with NO timeout anywhere. A stalled socket (proxy/firewall/AV) blocks the GPU- pool worker forever inside get_model() -- before the try/except that would surface an error -- and the frontend /generate fetch had no abort, so the spinner spun forever with no toast. - backend/main.py: set HF_HUB_ETAG_TIMEOUT=15 + HF_HUB_DOWNLOAD_TIMEOUT=30 (per-read timeout: resets on each chunk, so slow-but-progressing downloads are never punished; only a dead socket trips it). Set before hf import. - model_manager: get_model()/preload_model() now load via _load_model_with_timeout(), an asyncio.wait_for backstop (OMNIVOICE_MODEL_LOAD_TIMEOUT, default 1200s) that drops the poisoned GPU pool and raises a clear, actionable RuntimeError so a retry gets a fresh worker instead of queueing behind the wedged one. - useTTS.js: AbortController backstop on /generate so the UI never spins forever even if the backend is unreachable; friendly timeout toast. - tests: watchdog raises + resets pool + releases lock; env/floor parsing. Cross-platform (no OS-specific behavior); backward-compatible with installed models; local-first preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds timeout enforcement and pool cleanup to prevent model loading and TTS requests from hanging indefinitely. Hugging Face Hub network timeouts are set at startup, model load operations run with deadline enforcement and GPU pool reset on timeout, and frontend TTS requests abort after 21 minutes with cleanup. ChangesHang Prevention Infrastructure
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if pool is not None: | ||
| try: | ||
| pool.shutdown(wait=False, cancel_futures=True) | ||
| except Exception: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/hooks/useTTS.js`:
- Around line 157-159: The hardcoded timeout message assigned to msg in
useTTS.js must be routed through the i18n layer: replace the inline string for
the AbortError branch with a call to t('tts.timeout') and add the corresponding
key/value to the locales JSON files (e.g., locales/en.json) with the original
message text; ensure you import/use the existing t function in the same scope as
msg so the conditional becomes err?.name === 'AbortError' ? t('tts.timeout') :
("Error: " + err.message) and keep the non-timeout branch unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8419003a-69e1-46dd-85f5-0bc820311ec8
📒 Files selected for processing (4)
backend/main.pybackend/services/model_manager.pyfrontend/src/hooks/useTTS.jstests/test_model_load_timeout.py
| const msg = err?.name === 'AbortError' | ||
| ? 'Generation timed out — the model may still be downloading. Check Settings → Logs, then try again.' | ||
| : ("Error: " + err.message); |
There was a problem hiding this comment.
Route the new timeout toast through the i18n layer.
This newly added user-facing string is hardcoded rather than going through a t('...') key. Existing toasts in this file share the same pattern, but per the change-scoped guideline this new message should use the translation layer (e.g., t('tts.timeout')) with the copy living in locales/*.json.
As per coding guidelines: "All user-facing text in the UI must go through the i18n translation layer using t('...') keys in locales/*.json files, never hardcode non-English text".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/hooks/useTTS.js` around lines 157 - 159, The hardcoded timeout
message assigned to msg in useTTS.js must be routed through the i18n layer:
replace the inline string for the AbortError branch with a call to
t('tts.timeout') and add the corresponding key/value to the locales JSON files
(e.g., locales/en.json) with the original message text; ensure you import/use
the existing t function in the same scope as msg so the conditional becomes
err?.name === 'AbortError' ? t('tts.timeout') : ("Error: " + err.message) and
keep the non-timeout branch unchanged.
|
| Filename | Overview |
|---|---|
| backend/services/model_manager.py | Adds _load_model_with_timeout() wrapping run_in_executor in asyncio.wait_for; correctly resets GPU pool on timeout, but CancelledError from external cancellation bypasses _reset_gpu_pool(), leaving the pool poisoned until the watchdog fires. |
| backend/main.py | Adds HF_HUB_ETAG_TIMEOUT=15 and HF_HUB_DOWNLOAD_TIMEOUT=30 via os.environ.setdefault before huggingface_hub is imported; correctly user-overridable and cross-platform. |
| frontend/src/hooks/useTTS.js | Adds AbortController backstop on /generate with a hardcoded 21-minute timer and friendly timeout toast; generateSpeech correctly accepts and forwards the signal; magic constant not symbolically named. |
| tests/test_model_load_timeout.py | New regression tests for timeout and env-var floor; fixture correctly patches _model_lock and _load_model_sync, but doesn't manage _gpu_pool_singleton via monkeypatch so pool state leaks between tests. |
Sequence Diagram
sequenceDiagram
participant FE as Frontend (useTTS.js)
participant BE as /generate (FastAPI)
participant GM as get_model()
participant Pool as GPU ThreadPool
participant HF as HuggingFace Hub
FE->>+BE: "POST /generate (AbortController T=21min)"
BE->>+GM: get_model()
GM->>GM: acquire _model_lock
GM->>+Pool: run_in_executor(_load_model_sync)
Pool->>+HF: "from_pretrained() [HF_HUB_DOWNLOAD_TIMEOUT=30s per-read]"
alt Normal download
HF-->>Pool: model weights
Pool-->>GM: model loaded
GM-->>BE: model ready
BE-->>FE: WAV stream
FE->>FE: clearTimeout(abortTimer)
else Socket stall HF read timeout fires
HF--xPool: socket.timeout after 30s
Pool-->>GM: raises TimeoutError
GM-->>BE: error
BE-->>FE: HTTP error
FE->>FE: toast.error
else Watchdog fires asyncio.wait_for 1200s
Pool--xGM: asyncio.TimeoutError
GM->>GM: _reset_gpu_pool()
GM->>GM: release _model_lock
GM-->>BE: RuntimeError
BE-->>FE: HTTP 500
FE->>FE: toast.error
else Client abort backstop 21 min
FE->>FE: AbortController fires
FE-xBE: abort signal
FE->>FE: toast.error timeout
end
Reviews (1): Last reviewed commit: "fix(model): bound first-run model load/d..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async def _load_model_with_timeout(): | ||
| """Run the blocking model load on the GPU pool, bounded by a deadline. | ||
|
|
||
| Raises RuntimeError on timeout (and resets the poisoned pool) so callers | ||
| surface an actionable error instead of hanging indefinitely. | ||
| """ | ||
| loop = asyncio.get_running_loop() | ||
| timeout = _model_load_timeout() | ||
| try: | ||
| return await asyncio.wait_for( | ||
| loop.run_in_executor(_get_gpu_pool(), _load_model_sync), | ||
| timeout=timeout, | ||
| ) | ||
| except asyncio.TimeoutError as exc: | ||
| _set_loading("error", "Model load timed out", error="timeout") | ||
| _reset_gpu_pool() | ||
| logger.error("Model load exceeded %ss; resetting GPU pool.", timeout) | ||
| raise RuntimeError( | ||
| f"Model loading timed out after {int(timeout)}s — usually a network " | ||
| "stall downloading the model (proxy, firewall, or antivirus). Check " | ||
| "your connection or set a Hugging Face mirror in Settings, then retry." | ||
| ) from exc |
There was a problem hiding this comment.
CancelledError
_load_model_with_timeout only catches asyncio.TimeoutError. If the outer task is cancelled (e.g., the HTTP client disconnects mid-load, which uvicorn propagates as CancelledError), the exception bypasses the except asyncio.TimeoutError block entirely — _reset_gpu_pool() is never called. The poisoned single-worker pool persists, and the next /generate request silently queues behind the still-running stuck thread. The watchdog still fires eventually (after up to OMNIVOICE_MODEL_LOAD_TIMEOUT seconds), so it's not "hang forever" — but a retry that should be instant instead waits up to 20 minutes before the watchdog finally resets the pool.
| const ac = new AbortController(); | ||
| abortTimer = setTimeout(() => ac.abort(), 21 * 60 * 1000); |
There was a problem hiding this comment.
OMNIVOICE_MODEL_LOAD_TIMEOUT is overridden on the backend. A user on a very slow link who sets, e.g., OMNIVOICE_MODEL_LOAD_TIMEOUT=3600 will have the frontend silently abort after 21 minutes while the backend is still downloading. The magic constant should at least be defined symbolically so it's obvious it must track the backend value.
| const ac = new AbortController(); | |
| abortTimer = setTimeout(() => ac.abort(), 21 * 60 * 1000); | |
| const ac = new AbortController(); | |
| // Keep this ceiling above the backend's OMNIVOICE_MODEL_LOAD_TIMEOUT | |
| // (default 1200 s = 20 min) so the backend's descriptive error wins. | |
| const FRONTEND_ABORT_MS = 21 * 60 * 1000; // 21 min — must exceed backend default | |
| abortTimer = setTimeout(() => ac.abort(), FRONTEND_ABORT_MS); |
| monkeypatch.setattr(mm, "model", None) | ||
| return mm |
There was a problem hiding this comment.
mm.model via monkeypatch (auto-restored on teardown), but _gpu_pool_singleton is modified directly by _reset_gpu_pool() inside the test and monkeypatch never restores it. Subsequent tests start with no pool — self-healing via lazy init, but the teardown asymmetry is fragile for anyone adding pool-state assertions later.
| monkeypatch.setattr(mm, "model", None) | |
| return mm | |
| monkeypatch.setattr(mm, "model", None) | |
| monkeypatch.setattr(mm, "_gpu_pool_singleton", None) | |
| return mm |
2 participants
Bug
Windows users report: install succeeds, app runs, but creating the demo voice runs indefinitely — spinner forever, no audio, no error toast.
Root cause
The first
/generatecallsOmniVoice.from_pretrained(), which downloads the multi-GB weights via the legacy LFS path (HF_HUB_DISABLE_XET=1, set globally) with no timeout anywhere. A stalled socket (corporate proxy / firewall / antivirus inspecting the large transfer) blocks the GPU-pool worker insideget_model()— before thetry/exceptthat would surface an error — so it neither returns nor raises. The frontend/generatefetch (useTTS.js) had no abort/timeout, so the UI spun forever with no message.Fix (platform-agnostic, defense-in-depth)
backend/main.py):HF_HUB_ETAG_TIMEOUT=15+HF_HUB_DOWNLOAD_TIMEOUT=30. The download timeout is per-read — it resets on every received chunk, so a slow-but-progressing download is never punished; only a genuinely dead socket trips it (→ raises → surfaces as an error). Set beforehuggingface_hubimport.model_manager.py):get_model()/preload_model()load via_load_model_with_timeout(), anasyncio.wait_forbackstop (OMNIVOICE_MODEL_LOAD_TIMEOUT, default 1200s) that drops the poisoned single-worker GPU pool and raises a clear, actionableRuntimeErrorso a retry gets a fresh worker instead of queueing behind the wedged thread.useTTS.js):AbortControlleron/generateso the UI never spins forever even if the backend is unreachable; friendly timeout toast.Verify
Backend
pytest tests/test_model_load_timeout.py(2) +test_model_manager_preload.py(3) pass; frontend typecheck/build clean, vitest 103/103, legacy 36/0. On Windows, simulate a stalled HF download (unreachable proxy) → within the read-timeout the request now fails with a clear toast and the spinner clears, instead of hanging.Constraints: cross-platform parity (no OS-specific behavior), backward-compatible with already-installed models, local-first preserved.
🤖 Generated with Claude Code
Summary by CodeRabbit