fix(engines): path-aware GPU-pool slot in SubprocessBackend.generate() (on-pool skip + off-pool hold) by paoloantinori · Pull Request #1298 · debpalash/VoiceStudio

paoloantinori · 2026-07-29T07:45:15Z

Closes #1295, #1297. Supersedes #1296.

SubprocessBackend.generate()'s GPU-pool slot handling had two bugs, both fixed here in one path-aware slot block:

On-pool self-deadlock (Subprocess TTS engines deadlock on MPS (1-worker pool) via /v1/audio/speech and /generate #1295): /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.
Off-pool no hold (SubprocessBackend off-pool slot is a queue-wait, not a held reservation (over-subscribes a 1-worker GPU) #1297): 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.

Changed

generate() is now path-aware: on-pool callers skip the slot (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.

Tests / verification

generate dispatched on a pool worker (on-pool skip): pre-fix raised the ~10s slot timeout before the sidecar spawned.
a concurrent pool job blocked during an off-pool generate (off-pool hold): pre-fix the marker ran during the synth (slot not held).
Both verified fail-before / pass-after.

Notes

Supersedes fix(engines): stop SubprocessBackend.generate() self-deadlock on 1-worker (MPS) pools #1296 (on-pool-skip-only): this PR includes the skip plus the off-pool hold, so fix(engines): stop SubprocessBackend.generate() self-deadlock on 1-worker (MPS) pools #1296 can be closed.
The sibling services/subprocess_asr.py has the same no-op-reserve pattern; extracting a reserve_worker() helper on _ResilientGpuPool (used by both) is a clean follow-up, out of scope here.

Updated SubprocessBackend.generate() to avoid GPU-pool self-deadlocks by skipping nested slot acquisition when running_on_gpu_pool() is true, while off-pool callers now submit a blocking _occupy task that holds the single pool slot for the entire sidecar synthesis and releases it in a unified finally (with a 10s wait/TimeoutError if the slot can’t be acquired). Centralized detection of GPU pool worker threads in model_manager via running_on_gpu_pool(). Added regression coverage ensuring (1) on-pool invocation on a 1-worker pool completes without hanging and (2) off-pool generation keeps a second pool job blocked until synthesis finishes; main risk to review is the _occupy wait/timeout and future cancellation/cleanup correctness on all exit paths.

+job and over-subscribe a 1-worker GPU. This test reproduces that: while an
+off-pool generate is mid-synthesis, a second pool job must stay blocked.
+"""
+import base64


+off-pool generate is mid-synthesis, a second pool job must stay blocked.
+"""
+import base64
+import json


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


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


greptile-apps · 2026-07-29T07:47:42Z

Greptile Summary

This PR revises subprocess-engine GPU-slot handling.

Detects calls already executing on the GPU pool and skips nested reservation.
Holds a GPU worker for the full duration of off-pool subprocess synthesis.
Adds regression tests for on-pool deadlock avoidance and off-pool slot retention.

Important Files Changed

Filename	Overview
backend/services/model_manager.py	Centralizes the GPU-pool thread prefix and exposes pool-worker detection used by scheduling-sensitive paths.
backend/services/subprocess_backend.py	Implements path-aware slot ownership, but the previously reported ten-second off-pool acquisition timeout remains.
backend/tests/test_off_pool_slot_hold.py	Verifies that off-pool generation retains the sole GPU worker throughout synthesis.
backend/tests/test_subprocess_slot_deadlock.py	Verifies that generation already running on a single-worker GPU pool does not reserve a nested slot.

_{Reviews (5): Last reviewed commit: "Merge branch 'main' into fix/subprocess-..." | Re-trigger Greptile}

coderabbitai · 2026-07-29T07:48:19Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

GPU slot hold

Layer / File(s)	Summary
Reserve and release the GPU worker `backend/services/subprocess_backend.py`, `backend/services/model_manager.py`	`generate()` reserves a GPU-pool worker throughout off-pool synthesis, skips re-acquisition on pool workers, times out after 10 seconds if acquisition does not begin, and releases the reservation in `finally`.
Verify synthesis-time slot holding `backend/tests/test_off_pool_slot_hold.py`	A sleeping stub sidecar and one-worker pool test confirm queued pool work remains blocked until off-pool synthesis completes.
Verify pool-worker execution `backend/tests/test_subprocess_slot_deadlock.py`	A one-worker pool regression test confirms `generate()` completes when dispatched from a GPU-pool worker.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

debpalash/OmniVoice-Studio#1295 — Tracks the subprocess TTS deadlock fixed by skipping pool reacquisition on GPU-pool workers.
debpalash/OmniVoice-Studio#1297 — Addresses the same off-pool GPU-slot reservation behavior in SubprocessBackend.generate().

Possibly related PRs

debpalash/OmniVoice-Studio#1292 — Related SubprocessBackend.generate() sidecar and GPU-pool behavior.
debpalash/OmniVoice-Studio#1296 — Related pool-worker deadlock handling and regression coverage.
debpalash/OmniVoice-Studio#1204 — Related shared GPU-pool thread detection in model-manager placement logic.

🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% 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 uses conventional-commit scope and accurately describes the GPU-pool slot handling fix; the issue references are present in the body.
Description check	✅ Passed	It covers the summary, key changes, linked issues, and testing, so the template is mostly satisfied despite missing the exact section headings.
Linked Issues check	✅ Passed	The changes skip nested slot acquisition on-pool, hold a real slot off-pool, and add regression tests, matching `#1295`'s acceptance criteria.
Out of Scope Changes check	✅ Passed	The model_manager helper and both tests support the same slot-handling fix, so no unrelated changes stand out.
Cross-Platform Default Parity	✅ Passed	The new slot logic is OS-agnostic: one shared pool prefix and `running_on_gpu_pool()` gate are used on all platforms, with no macOS/Windows/Linux branch.
I18n Completeness (21 Locales)	✅ Passed	No frontend files changed in the PR, so there are no new t('...') keys or hardcoded user-facing strings to audit against the 21 locale files.
Local-First Guarantee	✅ Passed	Touched code only changes local GPU slot accounting and adds local regression tests; no new network, auth, telemetry, or required cloud calls appear in the PR.
Backward Compatibility	✅ Passed	Touched code only changes GPU-pool scheduling and tests; no DB schema, migration, or model-download/install paths changed, so existing data and installed engines stay compatible.

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: 2

🤖 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/services/subprocess_backend.py`:
- Around line 525-543: Update the reservation logic in generate() around _occupy
and slot_future to detect when execution is already running on the GPU pool
worker and bypass the additional pool.submit reservation, preserving the
existing in-pool path. Keep the reservation for off-pool callers, and add a
regression test covering reentrant generate() on a one-worker pool without the
ten-second timeout.

In `@backend/tests/test_off_pool_slot_hold.py`:
- Around line 113-129: Make the off-pool slot-hold test synchronize through an
instrumented reservation-start event instead of fixed sleeps, ensuring the
generator has reserved its slot before submitting and checking marker. Wrap the
test body in a finally block that always calls b.shutdown() and pool.shutdown(),
including when assertions or generation fail; preserve the existing assertions
and cleanup behavior otherwise.

🪄 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: deb4de28-fd7a-41cd-af9d-d6daae9225b9

📥 Commits

Reviewing files that changed from the base of the PR and between 574b283 and 970e2b1.

📒 Files selected for processing (2)

backend/services/subprocess_backend.py
backend/tests/test_off_pool_slot_hold.py

… 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.

+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


+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


/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.

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>

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>

+import array
+import sys
+import threading
+import time


# Conflicts: # backend/services/subprocess_backend.py

…ribe() (ASR sibling of #1298) (#1304) * fix(engines): path-aware GPU-pool slot in SubprocessASRBackend.transcribe() transcribe() had the same on-pool self-deadlock that generate() had (fixed in #1298): a bare no-op submitted to the GPU pool, but run_transcribe_guarded dispatches it via run_in_executor(_gpu_pool), already on a pool worker, so on a 1-worker (MPS) pool the no-op queued behind the job running it and timed out before the sidecar spawned. IsolatedFasterWhisperBackend on MPS hit this on every transcription. Mirror generate()'s path-aware slot block (on-pool skip via running_on_gpu_pool; off-pool _occupy hold) in transcribe(). The pattern is duplicated rather than extracted into a shared helper to avoid reworking generate(), which just shipped (#1298) with a CodeQL fix; extracting a shared contextmanager is a clean follow-up. Regression test added (transcribe dispatched on a pool worker). * fix(engines): import threading in subprocess_asr transcribe()'s off-pool slot hold uses threading.Event(), but the module never imported threading — every subprocess-ASR transcribe raised NameError, and the three round-trip tests failed in CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(engines): cover the off-pool transcribe branch Only the on-pool path had a test, so `threading.Event()` in the off-pool branch shipped with `threading` never imported — every direct caller hit NameError before the sidecar started. Both bots caught it on review; nothing in the suite did. A branch with no test is how a one-word bug reaches CI. Also asserts the slot is genuinely released afterwards. Fails without the import fix; the pre-existing on-pool test still passes. 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

greptile-apps Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread backend/services/subprocess_backend.py Outdated

coderabbitai Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread backend/services/subprocess_backend.py Outdated

Comment thread backend/tests/test_off_pool_slot_hold.py Outdated

paoloantinori force-pushed the fix/subprocess-off-pool-slot-hold branch from 970e2b1 to 9fdfc17 Compare July 29, 2026 08:05

paoloantinori changed the title ~~fix(engines): hold the GPU-pool slot for the whole off-pool synthesis~~ fix(engines): path-aware GPU-pool slot in SubprocessBackend.generate() (on-pool skip + off-pool hold) Jul 29, 2026

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

View reviewed changes

Comment thread backend/services/subprocess_backend.py Fixed

debpalash and others added 2 commits July 29, 2026 14:17

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

View reviewed changes

Comment thread backend/tests/test_off_pool_slot_hold.py

import array

import sys

import threading

import time

greptile-apps Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread backend/services/subprocess_backend.py

Merge branch 'main' into fix/subprocess-off-pool-slot-hold

54973cd

# Conflicts: # backend/services/subprocess_backend.py

debpalash merged commit 0e9733b into debpalash:main Jul 29, 2026
16 checks passed

coderabbitai Bot mentioned this pull request Aug 4, 2026

fix(gpu): record WHERE a wedged job was stuck when its budget expires (#1338) #1355

Merged

Navigation Menu

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

fix(engines): path-aware GPU-pool slot in SubprocessBackend.generate() (on-pool skip + off-pool hold)#1298
debpalash merged 5 commits into
debpalash:mainfrom
paoloantinori:fix/subprocess-off-pool-slot-hold

paoloantinori commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 29, 2026 •

edited

Loading

Greptile Summary

Important Files Changed

Uh oh!

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 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!

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!

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

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paoloantinori commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

greptile-apps Bot commented Jul 29, 2026 •

edited

Loading

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading