fix(engines): stop SubprocessBackend.generate() self-deadlock on 1-worker (MPS) pools by paoloantinori · Pull Request #1296 · debpalash/VoiceStudio

paoloantinori · 2026-07-29T06:01:59Z

Closes #1295.

SubprocessBackend.generate() acquires a GPU-pool slot for accounting, but /v1/audio/speech and /generate (and audiobook / dub / batch) dispatch backend.generate() via run_on_gpu_pool_guarded, already ON a pool worker. On a 1-worker pool (MPS) the inner pool.submit queues behind the very job running it and result(timeout=10) raises before the sidecar spawns, so every subprocess engine (IndexTTS-2, Supertonic-3, dots.tts, MOSS-TTS-v1.5, Confucius4) surfaces the in-process 300s-abandon instead of synthesizing.

Changed

generate() skips the slot acquisition when current_thread() is already a gpu-pool worker; the outer run_on_gpu_pool_guarded already accounts for the slot (it holds _running for the whole sidecar exchange via _tracked). Direct off-pool callers (the engine self-test in engines.py, the diagnostic probe in diagnose.py) still take a slot as a queue wait.

Tests / verification

New regression test: backend/tests/test_subprocess_slot_deadlock.py (generate dispatched on a pool worker; pre-fix it raised the ~10s slot timeout before the sidecar spawned).
Verified end-to-end on a live MPS server: /v1/audio/speech returns HTTP 200 with real audio, sidecar spawns.

Notes

Independent of feat(engines): add omnivoice-subprocess, a crash-isolated (killable) TTS engine #1292 (the omnivoice-subprocess feature, which carries the same fix at e9a15d53). This PR is the standalone general bug fix; if it merges first, feat(engines): add omnivoice-subprocess, a crash-isolated (killable) TTS engine #1292 rebases onto it.

Updated SubprocessBackend.generate() to skip the inner GPU-pool “slot” acquisition when the call already runs on a gpu-pool worker thread, preventing self-deadlock on one-worker pools (including MPS) while keeping slot accounting for direct off-pool callers. Added a regression test that monkeypatches _get_gpu_pool to a deterministic one-worker gpu-pool thread pool and uses a stub sidecar to assert generate() completes and returns the expected audio length without timing out. Risk is mainly the thread-name-based gpu-pool detection, which should be reviewed for brittleness across pool implementations.

…rker pools (debpalash#1295) generate() acquires a GPU-pool slot for accounting, but /v1/audio/speech and /generate (and audiobook/dub/batch) dispatch backend.generate() via run_on_gpu_pool_guarded, already ON a pool worker. On a 1-worker pool (MPS) the inner pool.submit queues behind the very job running it and result(timeout=10) raises before the sidecar spawns, so every subprocess engine surfaces the in-process 300s-abandon instead of synthesizing. Skip the slot acquisition when current_thread() is already a gpu-pool worker; the outer guard already accounts for the slot (holds _running for the whole sidecar exchange). Direct off-pool callers (engine self-test, diagnostic probe) still acquire one as a queue wait. Regression test: generate dispatched on a pool worker. Closes debpalash#1295.

+result(timeout=10) raised before the sidecar spawned. This test reproduces that
+dispatch shape (generate on a pool worker) against a stub sidecar.
+"""
+import base64


coderabbitai · 2026-07-29T06:03:55Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2bc4f85f-aeae-4d06-8701-929d7446ce0a

📥 Commits

Reviewing files that changed from the base of the PR and between 369816f and 20a2379.

📒 Files selected for processing (1)

backend/tests/test_subprocess_slot_deadlock.py

🚧 Files skipped from review as they are similar to previous changes (1)

backend/tests/test_subprocess_slot_deadlock.py

📝 Walkthrough

Walkthrough

SubprocessBackend.generate() skips nested GPU-pool slot acquisition when running on a GPU-pool worker. A regression test uses a stub sidecar and verifies generation completes with a 24,000-sample output.

Changes

Subprocess GPU-pool execution

Layer / File(s)	Summary
Conditional GPU-pool slot acquisition `backend/services/subprocess_backend.py`	`generate()` detects GPU-pool worker threads and bypasses nested slot acquisition while preserving off-pool handling.
Worker-thread regression coverage `backend/tests/test_subprocess_slot_deadlock.py`	A stub sidecar, minimal backend, and single-worker pool test validate generation and tensor length, with shutdown cleanup.
Estimated code review effort: 3 (Moderate)	~20 minutes

Possibly related issues

debpalash/OmniVoice-Studio#1291 — Both changes modify SubprocessBackend.generate() for subprocess and GPU-pool execution, although that issue focuses on worker recovery and memory handling.

Possibly related PRs

debpalash/OmniVoice-Studio#97 — Both modify GPU-pool slot acquisition in SubprocessBackend.generate().
debpalash/OmniVoice-Studio#1292 — Applies the same nested GPU-pool acquisition fix to another subprocess engine.

🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the bug fix and uses conventional-commit scope; the issue reference is present in the PR body.
Description check	✅ Passed	The PR description covers summary, changes, and testing, but it doesn't follow the repo's template sections like Type and Checklist.
Linked Issues check	✅ Passed	The fix skips slot reacquisition on gpu-pool workers and adds a regression test, matching `#1295`'s deadlock requirements.
Out of Scope Changes check	✅ Passed	The changes stay focused on the deadlock fix and regression test, with no unrelated code paths or feature work added.
Cross-Platform Default Parity	✅ Passed	No OS-specific default divergence found: the new gpu-pool thread check is used cross-platform, and MPS’s 1-worker sizing is pre-existing behavior.
I18n Completeness (21 Locales)	✅ Passed	PR touches only backend files; no frontend t('...') keys or hardcoded UI strings changed, so locale coverage is not applicable.
Local-First Guarantee	✅ Passed	Only a local threading gate and regression test were added; no new outbound network, telemetry, API-key, or cloud dependency appears.
Backward Compatibility	✅ Passed	Only runtime slot-acquisition logic changed; no DB/schema/migration or model-weight paths were touched, so existing omnivoice_data and installed models keep working.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

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 `@backend/tests/test_subprocess_slot_deadlock.py`:
- Around line 102-106: Update the test around _get_gpu_pool and the nested
generate() submission to create or monkeypatch a GPU pool configured with
exactly one worker before invoking pool.submit. Ensure the test uses that
one-worker pool rather than an ambient multi-worker singleton, while preserving
the existing timeout and result assertions.

🪄 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: 1de89e1a-69d1-4542-aa54-7b9f6f72961f

📥 Commits

Reviewing files that changed from the base of the PR and between 574b283 and 369816f.

📒 Files selected for processing (2)

backend/services/subprocess_backend.py
backend/tests/test_subprocess_slot_deadlock.py

greptile-apps · 2026-07-29T06:04:07Z

Greptile Summary

Prevents nested GPU-pool slot acquisition for subprocess synthesis already running on a pool worker.

Preserves slot waiting for direct off-pool callers.
Adds a one-worker regression test using a stub sidecar.

Important Files Changed

Filename	Overview
backend/services/subprocess_backend.py	Detects canonical GPU-pool workers and avoids the nested submission that deadlocked single-worker pools.
backend/tests/test_subprocess_slot_deadlock.py	Reproduces production dispatch on a single-worker pool and verifies successful sidecar synthesis.

_{Reviews (2): Last reviewed commit: "test: force a 1-worker gpu-pool in the s..." | Re-trigger Greptile}

…alash#1296) CodeRabbit: the test used the ambient pool singleton, which may have >1 worker on some hosts, so the pre-fix nested submit could succeed and the regression would pass vacuously. Monkeypatch _get_gpu_pool to a fresh 1-worker "gpu-pool"-prefixed pool so the self-deadlock reproduces deterministically. Verified fail-before (TimeoutError at ~10s with the fix reverted) / pass-after.

paoloantinori · 2026-07-29T06:22:49Z

CodeRabbit: addressed in 20a2379. The test now monkeypatches _get_gpu_pool to a fresh 1-worker gpu-pool-prefixed pool, so the self-deadlock reproduces deterministically regardless of the ambient pool's worker count. Verified fail-before (TimeoutError at ~10s with the fix reverted) and pass-after.

+dispatch shape (generate on a pool worker) against a stub sidecar.
+"""
+import base64
+import json


+"""
+import base64
+import json
+import math


+import base64
+import json
+import math
+import array


+import sys
+from pathlib import Path
+
+import pytest


… pool generate()'s slot handling had two bugs: 1. On-pool self-deadlock: /v1/audio/speech and /generate (and audiobook, dub, batch) dispatch generate() via run_on_gpu_pool_guarded, already on a pool worker, so the inner slot submit queued behind the very job running it on a 1-worker (MPS) pool and timed out before the sidecar spawned. Every subprocess engine surfaced the in-process 300s-abandon instead of synthesizing. 2. Off-pool no hold: the off-pool slot was a bare no-op that released the worker before _spawn(), so off-pool callers (engine self-test, diagnostics) could synthesize concurrently with a pool job and over-subscribe the GPU. Make the slot block path-aware: on-pool callers skip (the outer run_on_gpu_pool_guarded already holds _running for the whole sidecar exchange); off-pool callers hold a real slot for the whole synthesis via an _occupy task that blocks the worker until _held is set in the finally. Single release point in the finally. Regression tests: generate dispatched on a pool worker (on-pool skip) and a concurrent pool job blocked during an off-pool generate (off-pool hold). Both verified fail-before / pass-after. Supersedes debpalash#1296 (on-pool-skip-only). Closes debpalash#1295, debpalash#1297.

paoloantinori · 2026-07-29T08:06:01Z

Superseded by #1298, which folds the on-pool skip (this PR) together with the off-pool hold in one path-aware slot block. Closing in favor of the complete fix.

…) (on-pool skip + off-pool hold) (#1298) * fix(engines): make SubprocessBackend.generate() path-aware on the GPU pool generate()'s slot handling had two bugs: 1. On-pool self-deadlock: /v1/audio/speech and /generate (and audiobook, dub, batch) dispatch generate() via run_on_gpu_pool_guarded, already on a pool worker, so the inner slot submit queued behind the very job running it on a 1-worker (MPS) pool and timed out before the sidecar spawned. Every subprocess engine surfaced the in-process 300s-abandon instead of synthesizing. 2. Off-pool no hold: the off-pool slot was a bare no-op that released the worker before _spawn(), so off-pool callers (engine self-test, diagnostics) could synthesize concurrently with a pool job and over-subscribe the GPU. Make the slot block path-aware: on-pool callers skip (the outer run_on_gpu_pool_guarded already holds _running for the whole sidecar exchange); off-pool callers hold a real slot for the whole synthesis via an _occupy task that blocks the worker until _held is set in the finally. Single release point in the finally. Regression tests: generate dispatched on a pool worker (on-pool skip) and a concurrent pool job blocked during an off-pool generate (off-pool hold). Both verified fail-before / pass-after. Supersedes #1296 (on-pool-skip-only). Closes #1295, #1297. * Address review: couple on-pool skip to the pool prefix; fix comment /simplify + /code-review flagged that the on-pool skip keyed on the literal "gpu-pool" string, decoupled from _build_gpu_pool's thread_name_prefix. A rename would silently re-introduce the exact self-deadlock this PR fixes (and the tests can't catch it, since they hardcode the prefix). Centralise the prefix in _GPU_POOL_THREAD_PREFIX + a running_on_gpu_pool() helper, used by _build_gpu_pool, the skip in generate(), and _heal_tts_placement. Also fix the comment: the Settings engine self-test rejects subprocess-isolated engines with a 400, so the only real off-pool caller is the diagnose.py deep-synth probe. * fix(engines): bind slot_future before the off-pool branch CodeQL py/uninitialized-local-variable (error, blocking CI). `_held is not None` does imply slot_future was assigned, so the current code is correct — but the two are only coupled by convention, which the analyser cannot see and a third exit path would quietly break. Binds it to None up front and guards the cancel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(engines): make the slot-hold regression deterministic and leak-free CodeRabbit, valid on both counts. The test used sleep(0.8)/sleep(0.5) as synchronization — the tests/** contract forbids it, and on a slow runner the marker could be enqueued before the generator had reserved anything, so the assertion passed for the wrong reason. It now waits on an event signalled when the slot task actually starts, and asserts "did not run" via a result() timeout rather than a bare sleep. Cleanup moved into finally: an assertion failure used to leak the sidecar process and the pool thread into the rest of the session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: debpalash <4178343+debpalash@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

github-advanced-security AI found potential problems Jul 29, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread backend/tests/test_subprocess_slot_deadlock.py

github-advanced-security AI found potential problems Jul 29, 2026

View reviewed changes

paoloantinori mentioned this pull request Jul 29, 2026

fix(engines): path-aware GPU-pool slot in SubprocessBackend.generate() (on-pool skip + off-pool hold) #1298

Merged

paoloantinori closed this Jul 29, 2026

paoloantinori mentioned this pull request Jul 29, 2026

Subprocess TTS engines deadlock on MPS (1-worker pool) via /v1/audio/speech and /generate #1295

Closed

coderabbitai Bot mentioned this pull request Jul 29, 2026

fix(engines): path-aware GPU-pool slot in SubprocessASRBackend.transcribe() (ASR sibling of #1298) #1304

Merged

Navigation Menu

fix(engines): stop SubprocessBackend.generate() self-deadlock on 1-worker (MPS) pools - #1296

fix(engines): stop SubprocessBackend.generate() self-deadlock on 1-worker (MPS) pools#1296
paoloantinori wants to merge 2 commits into
debpalash:mainfrom
paoloantinori:fix/subprocess-generate-slot-deadlock

paoloantinori commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 29, 2026 •

edited

Loading

Greptile Summary

Important Files Changed

Uh oh!

paoloantinori commented Jul 29, 2026

Uh oh!

paoloantinori commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

paoloantinori commented Jul 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changed

Tests / verification

Notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Important Files Changed

Uh oh!

paoloantinori commented Jul 29, 2026

Uh oh!

paoloantinori commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

paoloantinori commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

greptile-apps Bot commented Jul 29, 2026 •

edited

Loading