From 5aa1677a25fbf2b946615427cda2dc8d7fb86d11 Mon Sep 17 00:00:00 2001 From: Jamie Pine <32987599+jamiepine@users.noreply.github.com> Date: Sun, 19 Apr 2026 19:27:42 -0700 Subject: [PATCH] fix(offline): guard inference paths with HF_HUB_OFFLINE (#503) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(offline): guard inference paths with HF_HUB_OFFLINE (#462) PR #443 wrapped the model *load* path with `force_offline_if_cached` so cached models don't phone home at startup. The context manager restores `HF_HUB_OFFLINE` on exit, which left inference paths (generate, transcribe, voice-prompt creation) unguarded — and `qwen_tts`, `mlx_audio`, and `transformers` perform lazy tokenizer/processor/config lookups during inference. With internet on, those lookups are near-instant and invisible; with internet off, `requests` hangs on DNS or connect until the network returns. This is exactly what users in #462 describe: model shows "Loaded", internet drops, generation "thinks" forever, internet comes back, generation completes. Chatterbox and LuxTTS don't exhibit this because their engine libs resolve everything through already-cached paths at load time. Fix: wrap each inference-sync body with `force_offline_if_cached(True, ...)`. Since inference only runs after a successful load, weights are known to be on disk, so `is_cached=True` is unconditional. Also adds the load-time guard that was missing from `qwen_custom_voice_backend.py` — CustomVoice previously had no offline protection at all. Paths patched: - PyTorchTTSBackend.create_voice_prompt (create_voice_clone_prompt) - PyTorchTTSBackend.generate (generate_voice_clone) - PyTorchSTTBackend.transcribe (Whisper generate + decoder-prompt-ids) - MLXTTSBackend.generate (mlx_audio generate, all branches) - MLXSTTBackend.transcribe (mlx_audio whisper generate) - QwenCustomVoiceBackend._load_model_sync + generate Does not address the secondary `check_model_inputs() missing 'func'` error reported in the same issue — that's a `transformers` 5.x version-skew bug on the install path, separate concern. Fixes #462. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(offline): mutate cached HF constants + threadsafe refcount Review feedback on the initial fix surfaced two real issues: 1. ``os.environ`` toggles alone don't flip offline mode. ``huggingface_hub.constants.HF_HUB_OFFLINE`` is read once at import time into a module-level bool; ``transformers.utils.hub._is_offline_mode`` mirrors that bool at its own import time. The hot paths (``_http._default_backend_factory`` in huggingface_hub, ``is_offline_mode`` in transformers) read the cached bools — not the env — so mutating only ``os.environ`` was a no-op. 2. Race condition on concurrent inference. Two threads running inside ``force_offline_if_cached`` via ``asyncio.to_thread`` could have thread A's ``finally`` strip thread B's offline protection mid-run. Rewrite the helper to: - mutate ``huggingface_hub.constants.HF_HUB_OFFLINE`` and ``transformers.utils.hub._is_offline_mode`` directly - refcount concurrent users under a single ``threading.RLock`` so a shared offline window is restored only when the last caller exits - still write ``os.environ`` for anything that reads it dynamically Also addresses the unused-variable ruff flag on the Whisper transcribe path (``audio, sr`` → ``audio, _sr``). New unit tests cover the cached-constant mutation, env propagation, no-op on ``is_cached=False``, nested contexts, and a threaded race where a slow thread must retain offline mode after a peer exits. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(offline): atomic entry rollback + tidy test assertions Review follow-up: - Wrap the `_offline_refcount == 0` setup in a try/except so any failure during the cached-constant mutation (including unexpected non-ImportError like RuntimeError or AttributeError from a half-initialized module) rolls back *all* partial state before re-raising. Without this, a mid-setup crash could leave `huggingface_hub.constants.HF_HUB_OFFLINE` mutated but the refcount at 0 — a persistent offline flag outliving the process. - Swap ruff-flagged Yoda comparisons in the new test file (SIM300) and add a module-level note warning that these tests mutate global state and are not safe under cross-process parallelism. Co-Authored-By: Claude Opus 4.7 (1M context) * test(offline): make concurrency test deterministic and bounded Replace the `sleep(0.15)` ordering hack with an explicit `threading.Event` the fast thread sets in `finally`. The slow thread waits on that event (bounded), then observes the flag — so we deterministically verify the slow thread still sees offline mode after the fast thread has exited. Also add timeouts to `barrier.wait()` and assert `not thread.is_alive()` after the joins so the test can't hang on an unexpected failure path. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- backend/backends/mlx_backend.py | 58 +++++--- backend/backends/pytorch_backend.py | 95 +++++++----- backend/backends/qwen_custom_voice_backend.py | 33 +++-- backend/tests/test_offline_guard.py | 118 +++++++++++++++ backend/utils/hf_offline_patch.py | 137 ++++++++++++++---- 5 files changed, 339 insertions(+), 102 deletions(-) create mode 100644 backend/tests/test_offline_guard.py diff --git a/backend/backends/mlx_backend.py b/backend/backends/mlx_backend.py index 5691cd50..ab54f536 100644 --- a/backend/backends/mlx_backend.py +++ b/backend/backends/mlx_backend.py @@ -193,6 +193,8 @@ class MLXTTSBackend: logger.info("Generating audio for text: %s", text) + model_name = f"qwen-tts-{self._current_model_size}" + def _generate_sync(): """Run synchronous generation in thread pool.""" # MLX generate() returns a generator yielding GenerationResult objects @@ -218,36 +220,40 @@ class MLXTTSBackend: logger.warning("Regenerating without voice prompt.") ref_audio = None - # Check if model supports voice cloning via generate method - # MLX API may support ref_audio parameter directly - try: - # Try with voice cloning parameters if supported - if ref_audio: - # Check if generate accepts ref_audio parameter - import inspect + # Model is loaded → weights are on disk. Force offline so + # lazy tokenizer/config lookups inside mlx_audio don't hang + # when the user is disconnected (issue #462). + with force_offline_if_cached(True, model_name): + # Check if model supports voice cloning via generate method + # MLX API may support ref_audio parameter directly + try: + # Try with voice cloning parameters if supported + if ref_audio: + # Check if generate accepts ref_audio parameter + import inspect - sig = inspect.signature(self.model.generate) - if "ref_audio" in sig.parameters: - # Generate with voice cloning - for result in self.model.generate(text, ref_audio=ref_audio, ref_text=ref_text, lang_code=lang): - audio_chunks.append(np.array(result.audio)) - sample_rate = result.sample_rate + sig = inspect.signature(self.model.generate) + if "ref_audio" in sig.parameters: + # Generate with voice cloning + for result in self.model.generate(text, ref_audio=ref_audio, ref_text=ref_text, lang_code=lang): + audio_chunks.append(np.array(result.audio)) + sample_rate = result.sample_rate + else: + # Fallback: generate without voice cloning + for result in self.model.generate(text, lang_code=lang): + audio_chunks.append(np.array(result.audio)) + sample_rate = result.sample_rate else: - # Fallback: generate without voice cloning + # No voice prompt, generate normally for result in self.model.generate(text, lang_code=lang): audio_chunks.append(np.array(result.audio)) sample_rate = result.sample_rate - else: - # No voice prompt, generate normally + except Exception as e: + # If voice cloning fails, try without it + logger.warning("Voice cloning failed, generating without voice prompt: %s", e) for result in self.model.generate(text, lang_code=lang): audio_chunks.append(np.array(result.audio)) sample_rate = result.sample_rate - except Exception as e: - # If voice cloning fails, try without it - logger.warning("Voice cloning failed, generating without voice prompt: %s", e) - for result in self.model.generate(text, lang_code=lang): - audio_chunks.append(np.array(result.audio)) - sample_rate = result.sample_rate # Concatenate all chunks if audio_chunks: @@ -341,6 +347,8 @@ class MLXSTTBackend: """ await self.load_model_async(model_size) + progress_model_name = f"whisper-{self.model_size}" + def _transcribe_sync(): """Run synchronous transcription in thread pool.""" # MLX Whisper transcription using generate method @@ -349,7 +357,11 @@ class MLXSTTBackend: if language: decode_options["language"] = language - result = self.model.generate(str(audio_path), **decode_options) + # Model is loaded → weights are on disk. Force offline so + # lazy tokenizer/config lookups don't hang when the user is + # disconnected (issue #462). + with force_offline_if_cached(True, progress_model_name): + result = self.model.generate(str(audio_path), **decode_options) # Extract text from result if isinstance(result, str): diff --git a/backend/backends/pytorch_backend.py b/backend/backends/pytorch_backend.py index b43a943e..ec66d5d5 100644 --- a/backend/backends/pytorch_backend.py +++ b/backend/backends/pytorch_backend.py @@ -172,13 +172,19 @@ class PyTorchTTSBackend: # This shouldn't happen in practice, but handle it return {"prompt": cached_prompt}, True + model_name = f"qwen-tts-{self._current_model_size}" + def _create_prompt_sync(): """Run synchronous voice prompt creation in thread pool.""" - return self.model.create_voice_clone_prompt( - ref_audio=str(audio_path), - ref_text=reference_text, - x_vector_only_mode=False, - ) + # Model is loaded → weights are on disk. Force offline so + # lazy tokenizer/config lookups inside qwen_tts don't hang + # when the user is disconnected (issue #462). + with force_offline_if_cached(True, model_name): + return self.model.create_voice_clone_prompt( + ref_audio=str(audio_path), + ref_text=reference_text, + x_vector_only_mode=False, + ) # Run blocking operation in thread pool voice_prompt_items = await asyncio.to_thread(_create_prompt_sync) @@ -221,19 +227,24 @@ class PyTorchTTSBackend: # Load model await self.load_model_async(None) + model_name = f"qwen-tts-{self._current_model_size}" + def _generate_sync(): """Run synchronous generation in thread pool.""" # Set seed if provided if seed is not None: manual_seed(seed, self.device) - # Generate audio - this is the blocking operation - wavs, sample_rate = self.model.generate_voice_clone( - text=text, - voice_clone_prompt=voice_prompt, - language=LANGUAGE_CODE_TO_NAME.get(language, "auto"), - instruct=instruct, - ) + # Model is loaded → weights are on disk. Force offline so + # lazy tokenizer/config lookups inside qwen_tts don't hang + # when the user is disconnected (issue #462). + with force_offline_if_cached(True, model_name): + wavs, sample_rate = self.model.generate_voice_clone( + text=text, + voice_clone_prompt=voice_prompt, + language=LANGUAGE_CODE_TO_NAME.get(language, "auto"), + instruct=instruct, + ) return wavs[0], sample_rate # Run blocking inference in thread pool to avoid blocking event loop @@ -331,40 +342,46 @@ class PyTorchSTTBackend: """ await self.load_model_async(model_size) + progress_model_name = f"whisper-{self.model_size}" + def _transcribe_sync(): """Run synchronous transcription in thread pool.""" # Load audio - audio, sr = load_audio(audio_path, sample_rate=16000) + audio, _sr = load_audio(audio_path, sample_rate=16000) - # Process audio - inputs = self.processor( - audio, - sampling_rate=16000, - return_tensors="pt", - ) - inputs = inputs.to(self.device) - - # Generate transcription - # If language is provided, force it; otherwise let Whisper auto-detect - generate_kwargs = {} - if language: - forced_decoder_ids = self.processor.get_decoder_prompt_ids( - language=language, - task="transcribe", + # Model is loaded → weights are on disk. Force offline so + # `get_decoder_prompt_ids` and any lazy tokenizer lookups + # don't hang when the user is disconnected (issue #462). + with force_offline_if_cached(True, progress_model_name): + # Process audio + inputs = self.processor( + audio, + sampling_rate=16000, + return_tensors="pt", ) - generate_kwargs["forced_decoder_ids"] = forced_decoder_ids + inputs = inputs.to(self.device) - with torch.no_grad(): - predicted_ids = self.model.generate( - inputs["input_features"], - **generate_kwargs, - ) + # Generate transcription + # If language is provided, force it; otherwise let Whisper auto-detect + generate_kwargs = {} + if language: + forced_decoder_ids = self.processor.get_decoder_prompt_ids( + language=language, + task="transcribe", + ) + generate_kwargs["forced_decoder_ids"] = forced_decoder_ids - # Decode - transcription = self.processor.batch_decode( - predicted_ids, - skip_special_tokens=True, - )[0] + with torch.no_grad(): + predicted_ids = self.model.generate( + inputs["input_features"], + **generate_kwargs, + ) + + # Decode + transcription = self.processor.batch_decode( + predicted_ids, + skip_special_tokens=True, + )[0] return transcription.strip() diff --git a/backend/backends/qwen_custom_voice_backend.py b/backend/backends/qwen_custom_voice_backend.py index fbbf9f30..bad5b3e2 100644 --- a/backend/backends/qwen_custom_voice_backend.py +++ b/backend/backends/qwen_custom_voice_backend.py @@ -28,6 +28,7 @@ from .base import ( combine_voice_prompts as _combine_voice_prompts, model_load_progress, ) +from ..utils.hf_offline_patch import force_offline_if_cached logger = logging.getLogger(__name__) @@ -104,18 +105,19 @@ class QwenCustomVoiceBackend: model_path = self._get_model_path(model_size) logger.info("Loading Qwen CustomVoice %s on %s...", model_size, self.device) - if self.device == "cpu": - self.model = Qwen3TTSModel.from_pretrained( - model_path, - torch_dtype=torch.float32, - low_cpu_mem_usage=False, - ) - else: - self.model = Qwen3TTSModel.from_pretrained( - model_path, - device_map=self.device, - torch_dtype=torch.bfloat16, - ) + with force_offline_if_cached(is_cached, model_name): + if self.device == "cpu": + self.model = Qwen3TTSModel.from_pretrained( + model_path, + torch_dtype=torch.float32, + low_cpu_mem_usage=False, + ) + else: + self.model = Qwen3TTSModel.from_pretrained( + model_path, + device_map=self.device, + torch_dtype=torch.bfloat16, + ) self._current_model_size = model_size self.model_size = model_size @@ -184,6 +186,7 @@ class QwenCustomVoiceBackend: await self.load_model_async(None) speaker = voice_prompt.get("preset_voice_id") or QWEN_CV_DEFAULT_SPEAKER + model_name = f"qwen-custom-voice-{self._current_model_size}" def _generate_sync(): if seed is not None: @@ -203,7 +206,11 @@ class QwenCustomVoiceBackend: if instruct: kwargs["instruct"] = instruct - wavs, sample_rate = self.model.generate_custom_voice(**kwargs) + # Model is loaded → weights are on disk. Force offline so + # lazy tokenizer/config lookups inside qwen_tts don't hang + # when the user is disconnected (issue #462). + with force_offline_if_cached(True, model_name): + wavs, sample_rate = self.model.generate_custom_voice(**kwargs) return wavs[0], sample_rate audio, sample_rate = await asyncio.to_thread(_generate_sync) diff --git a/backend/tests/test_offline_guard.py b/backend/tests/test_offline_guard.py new file mode 100644 index 00000000..5e3d2cd9 --- /dev/null +++ b/backend/tests/test_offline_guard.py @@ -0,0 +1,118 @@ +""" +Unit tests for the ``force_offline_if_cached`` helper. + +Verifies that the helper mutates the cached module constants in +``huggingface_hub.constants`` and ``transformers.utils.hub`` — not just +``os.environ`` — and that concurrent users are refcount-coordinated so +one thread's exit can't strip another thread's offline protection. + +NOTE: These tests mutate process-global state in ``huggingface_hub.constants`` +and ``transformers.utils.hub``. They are not safe under cross-process +parallelism (e.g. ``pytest-xdist`` with ``--dist=loadfile``/``loadscope``); +run this file serially. +""" + +import os +import sys +import threading +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from utils.hf_offline_patch import force_offline_if_cached # noqa: E402 + + +def _hf_const(): + import huggingface_hub.constants as hf_const + + return hf_const + + +def _tf_hub(): + import transformers.utils.hub as tf_hub + + return tf_hub + + +def test_mutates_cached_huggingface_hub_constant(): + original = _hf_const().HF_HUB_OFFLINE + with force_offline_if_cached(True, "t"): + assert _hf_const().HF_HUB_OFFLINE is True + assert original == _hf_const().HF_HUB_OFFLINE + + +def test_mutates_cached_transformers_constant(): + original = _tf_hub()._is_offline_mode + with force_offline_if_cached(True, "t"): + assert _tf_hub()._is_offline_mode is True + assert original == _tf_hub()._is_offline_mode + + +def test_sets_env_variable(): + original = os.environ.get("HF_HUB_OFFLINE") + with force_offline_if_cached(True, "t"): + assert "1" == os.environ.get("HF_HUB_OFFLINE") + assert original == os.environ.get("HF_HUB_OFFLINE") + + +def test_noop_when_not_cached(): + before = _hf_const().HF_HUB_OFFLINE + with force_offline_if_cached(False, "t"): + assert before == _hf_const().HF_HUB_OFFLINE + + +def test_nested_contexts_respect_refcount(): + original = _hf_const().HF_HUB_OFFLINE + with force_offline_if_cached(True, "outer"): + assert _hf_const().HF_HUB_OFFLINE is True + with force_offline_if_cached(True, "inner"): + assert _hf_const().HF_HUB_OFFLINE is True + # inner exit must not restore while outer is still active + assert _hf_const().HF_HUB_OFFLINE is True + assert original == _hf_const().HF_HUB_OFFLINE + + +def test_concurrent_threads_share_offline_window(): + """A slow thread must keep seeing offline mode even if a peer exits first.""" + original = _hf_const().HF_HUB_OFFLINE + observations: list[bool] = [] + errors: list[Exception] = [] + barrier = threading.Barrier(2) + fast_exited = threading.Event() + + def slow(): + try: + with force_offline_if_cached(True, "slow"): + barrier.wait(timeout=5) + assert fast_exited.wait(timeout=5), "fast thread did not exit" + observations.append(_hf_const().HF_HUB_OFFLINE) + except Exception as exc: # noqa: BLE001 + errors.append(exc) + + def fast(): + try: + with force_offline_if_cached(True, "fast"): + barrier.wait(timeout=5) + except Exception as exc: # noqa: BLE001 + errors.append(exc) + finally: + fast_exited.set() + + t_slow = threading.Thread(target=slow) + t_fast = threading.Thread(target=fast) + t_slow.start() + t_fast.start() + t_slow.join(timeout=5) + t_fast.join(timeout=5) + + assert not t_slow.is_alive(), "slow thread did not finish" + assert not t_fast.is_alive(), "fast thread did not finish" + assert not errors, errors + assert observations == [True], "slow thread lost offline protection" + assert original == _hf_const().HF_HUB_OFFLINE + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/backend/utils/hf_offline_patch.py b/backend/utils/hf_offline_patch.py index 1abd147f..354b6110 100644 --- a/backend/utils/hf_offline_patch.py +++ b/backend/utils/hf_offline_patch.py @@ -6,6 +6,7 @@ are already downloaded. Must be imported BEFORE mlx_audio. import logging import os +import threading from contextlib import contextmanager from pathlib import Path from typing import Optional, Union @@ -13,13 +14,33 @@ from typing import Optional, Union logger = logging.getLogger(__name__) +# huggingface_hub reads ``HF_HUB_OFFLINE`` once at import time into +# ``huggingface_hub.constants.HF_HUB_OFFLINE``; transformers mirrors that into +# ``transformers.utils.hub._is_offline_mode`` at *its* import time. Toggling +# ``os.environ`` after either module is imported does not flip those cached +# bools, and the hot paths (``_http._default_backend_factory``, +# ``transformers.utils.hub.is_offline_mode``) read the bools — not the env. +# We mutate the cached constants directly, guarded by a refcount so +# concurrent inference threads share a single offline window safely. + +_offline_lock = threading.RLock() +_offline_refcount = 0 +_saved_env: Optional[str] = None +_saved_hf_const: Optional[bool] = None +_saved_transformers_const: Optional[bool] = None + + @contextmanager def force_offline_if_cached(is_cached: bool, model_label: str = ""): - """Context manager that sets ``HF_HUB_OFFLINE=1`` while loading a cached model. + """Force offline mode for the duration of a cached-model operation. + + Flips ``HF_HUB_OFFLINE`` in the process env **and** in the cached bools + inside ``huggingface_hub.constants`` / ``transformers.utils.hub`` so HTTP + adapters and offline-mode checks actually see the change. Uses a refcount + so multiple concurrent inference threads share a single offline window + and the last one to exit restores state. If *is_cached* is ``False`` the block runs normally (network allowed). - If the offline load raises an error containing "offline" we automatically - retry with network access so a partially-cached model still works. Args: is_cached: Whether the model weights are already on disk. @@ -29,34 +50,96 @@ def force_offline_if_cached(is_cached: bool, model_label: str = ""): yield return - original_value = os.environ.get("HF_HUB_OFFLINE") - os.environ["HF_HUB_OFFLINE"] = "1" - logger.info( - "[offline-guard] %s is cached — forcing HF_HUB_OFFLINE=1", - model_label or "model", - ) + global _offline_refcount, _saved_env, _saved_hf_const, _saved_transformers_const + + with _offline_lock: + if _offline_refcount == 0: + # Snapshot prior state, apply new state, roll back on *any* + # failure. Catching only ImportError here would let a partially + # broken install (RuntimeError, AttributeError from a half-init + # module, etc.) leave the cached HF constants mutated without + # bumping the refcount — a persistent offline leak that outlives + # the process and is miserable to debug. + prev_env = os.environ.get("HF_HUB_OFFLINE") + prev_hf: Optional[bool] = None + prev_tf: Optional[bool] = None + try: + try: + import huggingface_hub.constants as hf_const + + prev_hf = hf_const.HF_HUB_OFFLINE + hf_const.HF_HUB_OFFLINE = True + except ImportError: + prev_hf = None + + try: + import transformers.utils.hub as tf_hub + + prev_tf = tf_hub._is_offline_mode + tf_hub._is_offline_mode = True + except ImportError: + prev_tf = None + + os.environ["HF_HUB_OFFLINE"] = "1" + except BaseException: + # Roll back whatever we already changed, then re-raise so + # the caller sees the real failure. + if prev_hf is not None: + try: + import huggingface_hub.constants as hf_const + + hf_const.HF_HUB_OFFLINE = prev_hf + except ImportError: + pass + if prev_tf is not None: + try: + import transformers.utils.hub as tf_hub + + tf_hub._is_offline_mode = prev_tf + except ImportError: + pass + if prev_env is not None: + os.environ["HF_HUB_OFFLINE"] = prev_env + else: + os.environ.pop("HF_HUB_OFFLINE", None) + raise + + _saved_env = prev_env + _saved_hf_const = prev_hf + _saved_transformers_const = prev_tf + logger.info( + "[offline-guard] %s is cached — forcing offline mode", + model_label or "model", + ) + _offline_refcount += 1 try: yield - except Exception as exc: - if "offline" in str(exc).lower(): - logger.warning( - "[offline-guard] Offline load failed for %s, retrying with network: %s", - model_label or "model", - exc, - ) - # Restore original env and retry — caller must wrap the load - # inside force_offline_if_cached so retrying here isn't possible. - # Instead, propagate a flag via the exception so the caller can - # decide. For simplicity we just let it fall through to the - # finally block and re-raise. - raise - raise finally: - if original_value is not None: - os.environ["HF_HUB_OFFLINE"] = original_value - else: - os.environ.pop("HF_HUB_OFFLINE", None) + with _offline_lock: + _offline_refcount -= 1 + if _offline_refcount == 0: + if _saved_env is not None: + os.environ["HF_HUB_OFFLINE"] = _saved_env + else: + os.environ.pop("HF_HUB_OFFLINE", None) + if _saved_hf_const is not None: + try: + import huggingface_hub.constants as hf_const + + hf_const.HF_HUB_OFFLINE = _saved_hf_const + except ImportError: + pass + if _saved_transformers_const is not None: + try: + import transformers.utils.hub as tf_hub + + tf_hub._is_offline_mode = _saved_transformers_const + except ImportError: + pass + _saved_env = None + _saved_hf_const = None + _saved_transformers_const = None def patch_huggingface_hub_offline():