fix(recognition): never lose a stop that races start, and read capture outside the buffer lock (#1023)

* fix(recognition): never lose a stop that races start, and read capture outside the buffer lock

start_recognition() published LISTENING (and played the start cue) before setting should_record. A stop arriving in that window, e.g. a push-to-talk release handled on another thread, was undone by the later should_record = True, leaving the capture thread recording with no way to stop it. The session is now armed under _buffer_lock before LISTENING is visible.

The capture loop held _buffer_lock across source.read_chunk(). When a read blocks (the audio server reconfigures the device, a USB/Bluetooth mic stalls), stop_recognition() gives up its bounded join and then blocks forever on that lock, stuck in PROCESSING. The read now happens outside the lock and the session is re-checked before the chunk is appended.

* test(recognition): wait for the stalled read before stopping

Signal from inside the blocking read instead of sleeping after the first, non-blocking one, so the test always exercises the stalled-read path.

* test(recognition): run the stalled-read test against the real NumPy

Older test modules leave sys.modules["numpy"] as a MagicMock. Under that leak the capture loop's first chunk raised before the stalled read was reached, so the test failed in the full-suite order on CI. Restore the real module for these tests, as test_playback_ducker.py already does.

---------

Co-authored-by: AAG Projects Team <293887937+aagprojectsteam-max@users.noreply.github.com>

src/vocalinux/speech_recognition/recognition_manager.py

@@ -3379,19 +3379,22 @@ class SpeechRecognitionManager:
             self._session_cancel = threading.Event()
             self._worker_exited = threading.Event()
             self._idle_on_worker_exit = None
+            # Arm the session before anyone can see LISTENING. A stop racing
+            # this start (a key released on another thread, the tray) acts as
+            # soon as it sees a non-IDLE state; setting should_record after
+            # that point would undo the stop and leave the capture thread
+            # recording with nothing left to stop it.
+            self.should_record = True
+            self._recognition_mode = mode
+            self.audio_buffer = []
+            self._segment_started_at = None
+            self._segment_queue = queue.Queue(maxsize=32)
+            self._key_release_gate = _ReleaseGate()
         self._update_state(RecognitionState.LISTENING)
 
         # Play the start sound
         play_start_sound()
 
-        # Set recording flag
-        self.should_record = True
-        self._recognition_mode = mode
-        self.audio_buffer = []
-        self._segment_started_at = None
-        self._segment_queue = queue.Queue(maxsize=32)
-        self._key_release_gate = _ReleaseGate()
-
         # Start the audio recording thread
         self.audio_thread = threading.Thread(target=self._record_audio)
         self.audio_thread.daemon = True
@@ -3828,7 +3831,16 @@ class SpeechRecognitionManager:
                             self.audio_buffer = self.audio_buffer[remove_count:]
                             logger.info(f"Buffer trimmed by {remove_count} chunks")
 
-                        data = source.read_chunk()
+                    # Read outside _buffer_lock. A capture read can block
+                    # indefinitely (the audio server reconfigures the device,
+                    # a USB/Bluetooth mic stalls); holding the lock across it
+                    # would block stop_recognition() on that same lock after
+                    # its bounded join, leaving the engine stuck in PROCESSING.
+                    data = source.read_chunk()
+
+                    with self._buffer_lock:
+                        # Re-check: the session may have been cancelled or
+                        # replaced while this thread was reading.
                         if session_cancel.is_set() or self._session_cancel is not session_cancel:
                             break
 

tests/test_start_stop_race.py

@@ -0,0 +1,155 @@
+"""Regression tests: a stop must never be lost, and a stalled mic must not hang it.
+
+1. A stop that arrives while start_recognition() is still running (key
+   released on another thread right after the state became LISTENING) used
+   to be undone by the later ``should_record = True``: the capture thread
+   then recorded with nothing left to stop it.
+2. A capture read that blocks (audio server reconfigures the device, a
+   USB/Bluetooth mic stalls) used to hold ``_buffer_lock``, so
+   stop_recognition() blocked forever on that lock after its bounded join.
+"""
+
+import sys
+import threading
+import time
+from typing import Iterator
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+import vocalinux.speech_recognition.recognition_manager as recognition_module
+from vocalinux.common_types import RecognitionState
+from vocalinux.speech_recognition.recognition_manager import SpeechRecognitionManager
+
+
+@pytest.fixture(autouse=True)
+def _numpy_is_real_while_capturing() -> Iterator[None]:
+    """Run the real capture loop against the real NumPy.
+
+    Older tests assign ``sys.modules["numpy"] = MagicMock()`` at import and
+    never put the real module back. ``_record_audio`` imports numpy lazily, so
+    under that leak its first chunk raises and the loop exits before the
+    stalled read this file depends on.
+    """
+    current = sys.modules.get("numpy")
+    real = getattr(sys, "_vocalinux_real_numpy", None)
+    if isinstance(current, MagicMock) and real is not None:
+        sys.modules["numpy"] = real
+        try:
+            yield
+        finally:
+            sys.modules["numpy"] = current
+    else:
+        yield
+
+
+def _make_manager() -> SpeechRecognitionManager:
+    duck = MagicMock()
+    duck.enabled.return_value = False
+    with patch.object(SpeechRecognitionManager, "_init_whispercpp"):
+        manager = SpeechRecognitionManager(
+            engine="whisper_cpp",
+            model_size="small",
+            language="en-us",
+            playback_duck=duck,
+        )
+    manager.model = MagicMock()
+    manager._model_initialized = True
+    manager._silero_vad = None
+    return manager
+
+
+def _stop_session(manager: SpeechRecognitionManager) -> None:
+    with patch.object(recognition_module, "play_stop_sound"):
+        with patch.object(SpeechRecognitionManager, "_STOP_WORKER_JOIN_TIMEOUT_S", 0.2):
+            with patch.object(SpeechRecognitionManager, "_STOP_WORKER_FINAL_JOIN_TIMEOUT_S", 0.05):
+                manager.stop_recognition()
+
+
+def _wait_for_state(manager: SpeechRecognitionManager, state: RecognitionState) -> None:
+    deadline = time.monotonic() + 3.0
+    while manager.state is not state and time.monotonic() < deadline:
+        time.sleep(0.005)
+    assert manager.state is state
+
+
+def test_stop_racing_start_is_not_lost() -> None:
+    manager = _make_manager()
+    stops: list[threading.Thread] = []
+
+    def record_audio() -> None:
+        # Stand-in for the capture loop: records while should_record is set.
+        deadline = time.monotonic() + 3.0
+        while manager.should_record and time.monotonic() < deadline:
+            time.sleep(0.005)
+
+    def release_key_on_another_thread(state: RecognitionState) -> None:
+        # The first LISTENING notification is the earliest point another
+        # thread can observe the session and stop it.
+        if state is RecognitionState.LISTENING and not stops:
+            stop = threading.Thread(target=_stop_session, args=(manager,))
+            stops.append(stop)
+            stop.start()
+            stop.join(timeout=5.0)
+
+    manager._record_audio = record_audio
+    manager.register_state_callback(release_key_on_another_thread)
+    try:
+        with patch.object(recognition_module, "play_start_sound"):
+            with patch.object(manager, "_arm_playback_duck"):
+                assert manager.start_recognition(mode="push_to_talk")
+
+        assert stops and not stops[0].is_alive()
+        # The stop must survive the rest of start_recognition().
+        assert manager.should_record is False
+        assert manager.audio_thread is not None
+        manager.audio_thread.join(timeout=1.0)
+        assert not manager.audio_thread.is_alive()
+        _wait_for_state(manager, RecognitionState.IDLE)
+    finally:
+        manager.should_record = False
+
+
+def test_stop_completes_while_capture_read_is_blocked() -> None:
+    manager = _make_manager()
+    first_read_done = threading.Event()
+    read_blocking = threading.Event()
+    release_read = threading.Event()
+
+    source = MagicMock()
+    source.requires_pyaudio = False
+    source.sample_rate = 16000
+
+    def read_chunk() -> bytes:
+        # The first read returns normally, every later one stalls like a
+        # device that disappeared under the audio server.
+        if first_read_done.is_set():
+            read_blocking.set()
+            release_read.wait(timeout=10.0)
+        first_read_done.set()
+        return b"\x00\x00" * 1024
+
+    source.read_chunk.side_effect = read_chunk
+    manager._new_capture_source = MagicMock(return_value=source)
+
+    try:
+        with patch.object(recognition_module, "play_start_sound"):
+            with patch.object(manager, "_arm_playback_duck"):
+                assert manager.start_recognition(mode="push_to_talk")
+        # Start the stop only once the capture thread is inside the stalled read.
+        assert read_blocking.wait(timeout=2.0)
+
+        stop = threading.Thread(target=_stop_session, args=(manager,), daemon=True)
+        stop.start()
+        # stop_recognition() joins the capture thread for at most 2 s; it
+        # must then finish instead of waiting on the buffer lock forever.
+        stop.join(timeout=5.0)
+        assert not stop.is_alive(), "stop_recognition() hung behind a blocked capture read"
+        _wait_for_state(manager, RecognitionState.IDLE)
+    finally:
+        release_read.set()
+        manager.should_record = False
+        if manager.audio_thread is not None:
+            manager.audio_thread.join(timeout=2.0)
+
+    assert not manager.audio_thread.is_alive()