Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions ovoscope/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@
from ovos_bus_client.session import SessionManager, Session
from ovos_config.config import Configuration
from ovos_config.models import LocalConf
from ovos_core.intent_services import IntentService

Check failure on line 12 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F401)

ovoscope/__init__.py:12:39: F401 `ovos_core.intent_services.IntentService` imported but unused help: Remove unused import: `ovos_core.intent_services.IntentService`
from ovos_core.skill_manager import SkillManager
from ovos_plugin_manager.skills import find_skill_plugins
from ovos_utils.fakebus import FakeBus
from ovos_utils.log import LOG
from ovos_utils.process_utils import ProcessState
from ovos_spec_tools import SpecMessage
from ovos_workshop.skills.ovos import OVOSSkill

SerializedMessage = Dict[str, Union[str, Dict[str, Any]]]
Expand Down Expand Up @@ -391,6 +392,23 @@
bus = FakeBus(modernize=self._modernize,
emit_legacy=self._emit_legacy)
bus.on("message", self.handle_boot_message)

# TTS mock: speak_dialog(…, wait=True) blocks on
# recognizer_loop:audio_output_end. Since there is no real TTS we
# schedule a short-delay emit to unblock the handler.
# This uses bus.ee.emit (not bus.emit) to bypass FakeBus's
# namespace-migration and on_message side effects so the synthetic
# event does not appear in test captures or reset session state.
def _mock_tts(message):
sess = SessionManager.get(message)
threading.Timer(0.1, lambda: bus.ee.emit(
"recognizer_loop:audio_output_end",
Message("recognizer_loop:audio_output_end",
context={"session": sess.serialize()})
)).start()

bus.on(SpecMessage.SPEAK, _mock_tts)

self.skill_ids = skill_ids
self.extra_skills = extra_skills or {}

Expand Down Expand Up @@ -732,7 +750,7 @@
###########################
track_bus_coverage: bool = False # enable BusCoverageTracker for this test
print_bus_coverage: bool = False # print inline summary after execute()
bus_coverage_report: Optional["BusCoverageReport"] = dataclasses.field(default=None, init=False, repr=False)

Check failure on line 753 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F821)

ovoscope/__init__.py:753:36: F821 Undefined name `BusCoverageReport`

###########################
# test runner internals
Expand Down Expand Up @@ -933,14 +951,14 @@
if self.verbose:
print(f"💡 final session: {last_sess.serialize()}")
print(f"> expected: {expected_sess.serialize()}")
assert {s[0] for s in last_sess.active_skills} == {s[0] for s in expected_sess.active_skills}, f"❌ final session active_skills doesn't match"

Check failure on line 954 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:954:108: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert sess.lang == expected_sess.lang, f"❌ final session lang doesn't match"

Check failure on line 955 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:955:53: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert sess.pipeline == expected_sess.pipeline, f"❌ final session pipeline doesn't match"

Check failure on line 956 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:956:61: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert sess.system_unit == expected_sess.system_unit, f"❌ final session system_unit doesn't match"

Check failure on line 957 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:957:67: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert sess.date_format == expected_sess.date_format, f"❌ final session date_format doesn't match"

Check failure on line 958 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:958:67: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert sess.time_format == expected_sess.time_format, f"❌ final session time_format doesn't match"

Check failure on line 959 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:959:67: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert sess.site_id == expected_sess.site_id, f"❌ final session site_id doesn't match"

Check failure on line 960 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:960:59: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert sess.session_id == expected_sess.session_id, f"❌ final session session_id doesn't match"

Check failure on line 961 in ovoscope/__init__.py

View workflow job for this annotation

GitHub Actions / lint / lint

ruff (F541)

ovoscope/__init__.py:961:65: F541 f-string without any placeholders help: Remove extraneous `f` prefix
assert set(sess.blacklisted_skills or []) == set(expected_sess.blacklisted_skills or []), f"❌ final session blacklisted_skills doesn't match"
assert set(sess.blacklisted_intents or []) == set(expected_sess.blacklisted_intents or []), f"❌ final session blacklisted_intents doesn't match"
if self.verbose:
Expand Down
20 changes: 20 additions & 0 deletions ovoscope/audio.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,26 @@ def reset(self) -> None:
"""Clear the list of recorded spoken utterances."""
self.spoken_utterances.clear()

def __del__(self) -> None:
"""No-op destructor.

``TTS.__del__`` chains into ``TTS.shutdown() -> TTS.stop() ->
TTS.playback.stop()``. ``TTS.playback`` is a **class-level** attribute
shared by every TTS instance in the process, so when an earlier
harness's MockTTS is garbage-collected its inherited destructor stops
whatever PlaybackThread is *currently* registered there — which, by
then, belongs to a later, still-running harness. The victim thread sets
``_terminated`` and exits mid-run, so its queued speak never plays and
``ovos.audio.output.ended`` is never emitted, hanging the next
``speak()`` wait.

GC timing is nondeterministic, so the failure surfaces as a flaky
``TimeoutError`` only after several harness instances have been created
and collected. The harness already manages thread lifecycle explicitly
via ``PlaybackService.shutdown()`` on context exit, so a MockTTS
instance must never tear down the shared playback thread on collection.
"""


# ---------------------------------------------------------------------------
# PlaybackServiceHarness
Expand Down
74 changes: 74 additions & 0 deletions test/unittests/test_audio_harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from ovos_utils.fakebus import FakeBus

if AUDIO_AVAILABLE:
from ovos_plugin_manager.templates.tts import TTS
from ovoscope.audio import (
AudioCaptureSession,
AudioServiceHarness,
Expand Down Expand Up @@ -493,5 +494,78 @@ def test_speak_lifecycle_via_bridging(self) -> None:
h.assert_audio_output_ended()


@unittest.skipUnless(AUDIO_AVAILABLE, "ovos-audio (audio extra) not installed")
class TestPlaybackServiceHarnessIsolation(unittest.TestCase):
"""Repeated, independent harness instances must not interfere.

Regression for the shared ``TTS.playback`` class-attribute hazard: a
garbage-collected MockTTS from an earlier harness used to stop the
PlaybackThread of a *later*, still-running harness (via the inherited
``TTS.__del__`` -> ``TTS.stop`` -> ``TTS.playback.stop()`` chain). The
victim thread terminated mid-run, its queued speak never played, and the
next ``speak()`` hung until timeout. Because GC timing is nondeterministic
this manifested as a flaky ``TimeoutError`` only after several
create/destroy cycles.
"""

def test_many_sequential_harnesses_each_complete_speaks(self) -> None:
"""Boot and tear down many harnesses, forcing GC between them, and
require every speak in every harness to complete deterministically."""
import gc

for i in range(12):
with PlaybackServiceHarness() as h:
for tag in ("a", "b", "c"):
# unique sentences so the persistent TTS cache never
# short-circuits synthesis — each must drive real playback
h.speak(f"iter {i} part {tag}", timeout=8.0)
self.assertIn(f"iter {i} part {tag}",
h.mock_tts.spoken_utterances)
# provoke collection of the just-exited MockTTS *now*, while a
# fresh harness will shortly own TTS.playback. Pre-fix, this is
# exactly what killed the next harness's playback thread.
gc.collect()

def test_stale_mock_destructor_does_not_kill_live_thread(self) -> None:
"""A finished harness's MockTTS destructor must not terminate the
playback thread that a *later* harness now owns.

Deterministic reproduction of the GC race: keep a reference to harness
A's MockTTS so it outlives A, open harness B (which registers its own
thread on the shared ``TTS.playback`` class attribute), then run A's
destructor. Pre-fix, ``MockTTS.__del__`` chained into
``TTS.playback.stop()`` and terminated B's live thread; B's next speak
would then hang. With the no-op destructor, B is unaffected.
"""
# Harness A — produce a MockTTS that survives the context exit.
with PlaybackServiceHarness() as ha:
ha.speak("harness A warmup", timeout=8.0)
stale_mock = ha.mock_tts

# Harness B now owns the shared TTS.playback thread.
with PlaybackServiceHarness() as hb:
self.assertIs(TTS.playback, hb.svc.playback_thread)
self.assertTrue(hb.svc.playback_thread.is_alive())

# Fire harness A's destructor explicitly (what GC would do).
stale_mock.__del__()

# The precise invariant: A's destructor must not have flagged B's
# thread for termination. ``_terminated`` is checked at the top of
# the playback loop, so a single in-flight speak can still slip
# through even when set — but the thread would then exit on its next
# iteration, hanging a subsequent speak. Assert the flag directly.
self.assertFalse(
hb.svc.playback_thread._terminated,
"stale MockTTS destructor terminated the live playback thread",
)

# And B must keep working across multiple speaks (the loop must not
# have exited).
for n in range(3):
hb.speak(f"harness B speak {n}", timeout=8.0)
self.assertTrue(hb.svc.playback_thread.is_alive())


if __name__ == "__main__":
unittest.main()
Loading