fix: MockTTS destructor must not stop the shared playback thread by JarbasAl · Pull Request #100 · OpenVoiceOS/ovoscope

JarbasAl · 2026-06-27T23:34:34Z

Problem

TTS.playback is a class-level attribute shared by every TTS instance in the process. The inherited TTS.__del__ chains into TTS.shutdown() -> TTS.stop() -> TTS.playback.stop(). So when an earlier PlaybackServiceHarness's MockTTS is garbage-collected, its destructor stops whatever PlaybackThread is currently registered there — which, by then, belongs to a later, still-running harness.

The victim thread gets _terminated set and exits its run loop, so its queued speak never plays and ovos.audio.output.ended is never emitted. The next speak() then hangs until timeout. Because GC timing is nondeterministic, this surfaced as a flaky TimeoutError only after several harness create/destroy cycles.

This was caught when ovos-audio wired up the ovoscope CI job (OpenVoiceOS/ovos-audio#171): its test/end2end/ suite went red with TimeoutError: Speech playback for '...' did not finish within 10.0s at ovoscope/audio.py (the speak() wait), on pre-existing tests that simply do 2+ speaks per harness after enough preceding harnesses.

Root cause (traced)

TTS.__del__  ->  TTS.shutdown  ->  TTS.stop  ->  TTS.playback.stop()   # class-level!

At the stall, the active harness's playback_thread.is_alive() == False / _terminated == True, with a speak item still queued and no live consumer.

Fix

Override MockTTS.__del__ as a no-op. The harness already manages the playback-thread lifecycle explicitly via PlaybackService.shutdown() on context exit, so a mock instance must never tear down the shared thread on collection.

Tests

test_stale_mock_destructor_does_not_kill_live_thread — deterministic guard: fire a stale mock's destructor while a later harness owns TTS.playback; assert the live thread is neither _terminated nor unable to keep speaking. Fails on dev, passes with the fix.
test_many_sequential_harnesses_each_complete_speaks — many create/destroy cycles with forced GC; every speak must complete.

Verification

	ovos-audio `test/end2end/test_playback_service_extended_e2e.py`
dev (unfixed)	`1 failed, 12 passed` (3/3 runs)
this PR	`13 passed` (3/3 runs)

Full ovoscope audio harness unit suite: 44 passed. Full ovos-audio test/end2end/: 57 passed (was 2 failed, 55 passed).

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved audio playback reliability by preventing cleanup from stopping active playback in later sessions.
- Reduced flaky timeouts when using audio features across repeated start/stop cycles.
Tests
- Added regression coverage for repeated playback lifecycle scenarios and stale-object cleanup behavior.

TTS.playback is a class-level attribute shared by every TTS instance in the process. The inherited TTS.__del__ chains into TTS.stop() -> TTS.playback.stop(), so when an earlier PlaybackServiceHarness's MockTTS is garbage-collected its destructor terminated whatever PlaybackThread was *currently* registered there — which by then belongs to a later, still-running harness. The victim thread had _terminated set and exited its loop, so its queued speak never played and ovos.audio.output.ended was never emitted, hanging the next speak() until timeout. GC timing made this a flaky TimeoutError that surfaced only after several harness create/destroy cycles (e.g. mid-file in a consumer's test/end2end suite). Override MockTTS.__del__ as a no-op: the harness already manages playback-thread lifecycle explicitly via PlaybackService.shutdown() on context exit, so a mock instance must never tear down the shared thread on collection. Add regression tests: a deterministic guard that fires a stale mock's destructor while a later harness owns TTS.playback and asserts the live thread is neither terminated nor unable to keep speaking, plus a many-sequential-harnesses smoke test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai · 2026-06-27T23:34:40Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c54779cd-3089-484a-a377-da15eecca33d

📥 Commits

Reviewing files that changed from the base of the PR and between 68f5837 and af751d6.

📒 Files selected for processing (2)

ovoscope/audio.py
test/unittests/test_audio_harness.py

📝 Walkthrough

Walkthrough

MockTTS gains an explicit no-op __del__ to suppress the inherited TTS.__del__ teardown, which could stop the shared class-level playback thread during garbage collection. Two regression tests are added to verify that sequential harness cycles and stale destructor calls do not kill the live thread or cause speak() timeouts.

Changes

MockTTS destructor isolation fix

Layer / File(s)	Summary
MockTTS no-op `__del__` and regression tests `ovoscope/audio.py`, `test/unittests/test_audio_harness.py`	`MockTTS.__del__` is overridden as a no-op with a docstring explaining why. A `TTS` import is added and `TestPlaybackServiceHarnessIsolation` is introduced with two regression tests: one exercising repeated harness create/teardown cycles under `gc.collect()`, and another deterministically invoking a stale `MockTTS.__del__` while a live harness holds the shared playback thread, asserting the thread is not terminated and speaks still complete.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

OpenVoiceOS/ovoscope#44: Originally introduced MockTTS and the audio harness that this PR directly patches.
OpenVoiceOS/ovoscope#75: Adds TTSIntelligibilityHarness which relies on PlaybackServiceHarness/MockTTS lifecycle and is directly affected by the shared playback thread behavior fixed here.

Poem

🐇 A ghost of __del__ lurked in the night,
Killing threads and dimming TTS light.
But now MockTTS says "nope, not today,"
A no-op wave sends that destructor away.
Speaks complete, the playback thread stays bright! 🎵

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/mocktts-del-shared-playback

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

github-actions · 2026-06-27T23:35:18Z

Checking in! Here's how the automated tests are looking. 🧐

I've aggregated the results of the automated checks for this PR below.

🔍 Lint

The automated pipeline is running smoothly. 🚂

❌ ruff: issues found — see job log

🏷️ Release Preview

The release announcement is being drafted in multiple languages. 🗣️

Current: 1.0.1a1 → Next: 1.0.2a1

Signal	Value
Label	(none)
PR title	`fix: MockTTS destructor must not stop the shared playback thread`
Bump	build

✅ PR title follows conventional commit format.

🚀 Release Channel Compatibility

Predicted next version: 1.0.2a1

Channel	Status	Note	Current Constraint
Stable	⚪	Not in channel	-
Testing	❌	Too new (must be <1.0.0)	`ovoscope>=0.7.2,<1.0.0`
Alpha	✅	Compatible	`ovoscope>=1.0.1a1`

📋 Repo Health

Ensuring the repo isn't allergic to new features. 🤧

✅ All required files present.

Latest Version: 1.0.1a1

✅ ovoscope/version.py — Version file
✅ README.md — README
✅ LICENSE — License file
✅ pyproject.toml — pyproject.toml
⚠️ setup.py — setup.py
✅ CHANGELOG.md — Changelog
✅ ovoscope/version.py has valid version block markers

🔒 Security (pip-audit)

I've audited the packages. Safety first! 🦺

✅ No known vulnerabilities found (77 packages scanned).

⚖️ License Check

Ensuring our project is well-protected legally. 🛡️

✅ No license violations found.

Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed.

🔨 Build Tests

I've fired up the furnaces and forged your changes. ⚒️

✅ All versions pass

Python	Build	Install	Tests
3.10	✅	✅	✅
3.11	✅	✅	✅
3.12	✅	✅	✅
3.13	✅	✅	✅
3.14	✅	✅	✅

📊 Coverage

Coverage report incoming! Every line counts. 🎯

❌ 53.2% total coverage

Files below 80% coverage (17 files)

File	Coverage	Missing lines
`ovoscope/simple_listener.py`	0.0%	55
`ovoscope/tts_intelligibility.py`	0.0%	190
`ovoscope/version.py`	0.0%	5
`ovoscope/intent_cases.py`	14.0%	141
`ovoscope/classic_listener.py`	18.2%	117
`ovoscope/pipeline.py`	31.0%	78
`ovoscope/ocp.py`	31.8%	58
`ovoscope/cli.py`	32.8%	162
`ovoscope/e2e.py`	34.9%	99
`ovoscope/pytest_plugin.py`	41.4%	222
`ovoscope/listener.py`	56.2%	123
`ovoscope/media.py`	57.2%	101
`ovoscope/voice_loop.py`	58.7%	114
`ovoscope/__init__.py`	58.8%	309
`ovoscope/coverage.py`	63.3%	69
`ovoscope/audio.py`	64.2%	112
`ovoscope/media_provider.py`	64.9%	20

Full report: download the coverage-report artifact.

Powered by OVOS scripts and a bit of magic. ✨

ovoscope 1.0.2a1 ships the MockTTS.__del__ no-op fix (OpenVoiceOS/ovoscope#100): a garbage-collected mock from an earlier PlaybackServiceHarness no longer stops the shared TTS.playback thread owned by a later harness, which had been flaking the test/end2end/ suite (TimeoutError on the 2nd+ speak). Floor-pinned (>=) so pip resolves the prerelease with no --pre. Full test/end2end/ now green (57/57). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat: adopt AUDIO-1 spec output topics (dual-namespace) Subscribe to the OVOS-AUDIO-1 spec-named topics additively, alongside the legacy mycroft.*/speak:* names, so both namespaces drive the same handlers: - ovos.utterance.speak.b64 (SpecMessage.SPEAK_B64, §3.4) + legacy speak:b64_audio - ovos.audio.queue (SpecMessage.AUDIO_QUEUE, §4.1) + legacy mycroft.audio.queue - ovos.audio.play_sound (SpecMessage.AUDIO_PLAY_SOUND, §4.2) + legacy mycroft.audio.play_sound - ovos.audio.is_speaking (SpecMessage.AUDIO_IS_SPEAKING, §5.3) + legacy mycroft.audio.speak.status - ovos.audio.stop (SpecMessage.AUDIO_STOP, §6) + legacy mycroft.audio.speech.stop The b64 handler now also emits ovos.audio.speech (SpecMessage.AUDIO_SPEECH, §3.4/§4.3) with the synthesised audio and ovos.mic.listen on listen=True, in addition to the legacy correlated .response. The speaking-status handler replies on ovos.audio.is_speaking and guards against answering its own reply (shared query/reply topic name). The universal ovos.stop broadcast stays wired (§6). Floor-pin ovos-spec-tools>=0.16.1a2 (first release carrying the six new SpecMessage members). Tests drive the service over a FakeBus and assert each of the six topics works under BOTH the legacy and the spec name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: don't hand-emit the legacy b64 reply — the bus client mirrors it handle_b64_audio emitted both ovos.audio.speech (AUDIO_SPEECH) AND the legacy speak:b64_audio.response (via message.response). The bus client's emit_legacy flag already mirrors ovos.audio.speech onto speak:b64_audio.response (ovos-spec-tools MIGRATION_MAP), so the legacy reply went on the wire twice. Emit the spec topic only and let the migration bridge legacy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: bump bus-client + spec-tools floors for AUDIO-1 dual-namespace handle_b64_audio relies on the bus-client emit_legacy migration and the AUDIO MIGRATION_MAP entries (added to ovos-spec-tools in the 0.17.x line, PR#55). Pin prerelease floors that guarantee the dual-namespace behavior is present, resolved by pip without --pre: ovos_bus_client >=2.2.0a1 -> >=2.5.1a1,<3.0.0 ovos-spec-tools >=0.16.1a2 -> >=0.17.3a1,<1.0.0 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: ovoscope e2e for b64 synthesised-audio dual-namespace delivery OVOS-AUDIO-1 §3.4/§4.3: handle_b64_audio now emits the synthesised audio on the spec topic ovos.audio.speech only; the bus client mirrors it onto the legacy speak:b64_audio.response (ovos-spec-tools MIGRATION_MAP). Add an additive ovoscope end-to-end test booting a real PlaybackService over a real bus with an offline MockTTS, driving both the legacy (speak:b64_audio) and the spec (ovos.utterance.speak.b64) input topics, and asserting the reply reaches a subscriber on BOTH ovos.audio.speech and speak:b64_audio.response with the expected b64 payload. Also covers listen=True re-opening the mic via ovos.mic.listen. Floor the ovoscope test dep to >=1.0.1a1 (first line whose transitive ovos-core no longer caps ovos-bus-client<2.0.0) and wire the gh-automations ovoscope.yml end-to-end workflow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: bump ovoscope floor to >=1.0.2a1 for the MockTTS shutdown fix ovoscope 1.0.2a1 ships the MockTTS.__del__ no-op fix (OpenVoiceOS/ovoscope#100): a garbage-collected mock from an earlier PlaybackServiceHarness no longer stops the shared TTS.playback thread owned by a later harness, which had been flaking the test/end2end/ suite (TimeoutError on the 2nd+ speak). Floor-pinned (>=) so pip resolves the prerelease with no --pre. Full test/end2end/ now green (57/57). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

github-actions Bot added the fix label Jun 27, 2026

JarbasAl marked this pull request as ready for review June 27, 2026 23:40

JarbasAl merged commit 5afdc72 into dev Jun 27, 2026
14 checks passed

JarbasAl deleted the fix/mocktts-del-shared-playback branch June 27, 2026 23:40

github-actions Bot added fix and removed fix labels Jun 27, 2026

coderabbitai Bot mentioned this pull request Jun 29, 2026

feat: MockTTS — emit audio_output_end on delay for speak_dialog(wait=True) #102

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: MockTTS destructor must not stop the shared playback thread#100

fix: MockTTS destructor must not stop the shared playback thread#100
JarbasAl merged 1 commit into
devfrom
fix/mocktts-del-shared-playback

JarbasAl commented Jun 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 27, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

github-actions Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JarbasAl commented Jun 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause (traced)

Fix

Tests

Verification

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

github-actions Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checking in! Here's how the automated tests are looking. 🧐

🔍 Lint

🏷️ Release Preview

📋 Repo Health

🔒 Security (pip-audit)

⚖️ License Check

🔨 Build Tests

📊 Coverage

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

JarbasAl commented Jun 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 27, 2026 •

edited

Loading

github-actions Bot commented Jun 27, 2026 •

edited

Loading