Fix race in nvdaController_speakSsml where speechCanceled could be missed#20220
Merged
Conversation
…ssed When asynchronous=False, onSpeechCanceled was registered inside prefixCallback, a CallbackCommand that runs on the synth thread after the sequence starts. A cancel firing between queueFunction returning and the synth reaching that CallbackCommand would not be caught, leaving markQueue.get() blocked indefinitely. Move speech.speechCanceled.register(onSpeechCanceled) to just before queueFunction so the handler exists before any cancel can fire. onDoneSpeaking stays in prefixCallback because synthDoneSpeaking fires per sequence; registering it earlier would catch a previous sequence still finishing when priority is Next. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates NVDAHelper’s SSML speak path to avoid a deadlock when nvdaController_speakSsml(..., asynchronous=False) is canceled during a narrow timing window, and documents the fix in the user-facing changelog.
Changes:
- Register
speech.speechCanceledearlier for synchronous SSML speech to eliminate a race that could cause indefinite blocking. - Update release notes to mention the fixed race condition/deadlock in
nvdaController_speakSsml.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| user_docs/en/changes.md | Adds a changelog entry describing the synchronous SSML cancellation race fix. |
| source/NVDAHelper/init.py | Moves speechCanceled handler registration earlier when asynchronous is False to prevent a deadlock. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
None
Summary of the issue:
When
nvdaController_speakSsmlis called withasynchronous=False, a race condition can cause the call to block indefinitely.onSpeechCanceledwas registered insideprefixCallback, aCallbackCommandthat runs on the synth thread only after the sequence starts. IfspeechCanceledfires in the window betweenqueueHandler.queueFunctionreturning and the synth reaching thatCallbackCommand, the signal is missed andmarkQueue.get()never unblocks.Description of user facing changes:
No change under normal operation. Fixes a hang that could occur when speech is cancelled immediately after being queued via the controller client.
Description of developer facing changes:
speech.speechCanceled.register(onSpeechCanceled)is now called directly beforequeueHandler.queueFunction, ensuring the handler exists before any cancel signal can fire.onDoneSpeakingstays inprefixCallbackbecausesynthDoneSpeakingfires per sequence — registering it earlier would catch a previous sequence still finishing when priority isNext.Description of development approach:
Minimal repositioning of one
registercall. No logic changes.Testing strategy:
Call
nvdaController_speakSsmlwithasynchronous=Falseand cancel speech immediately after queuing, repeatedly. Before this fix the call would occasionally hang; after it should always return promptly withERROR_CANCELLED.Known issues with pull request:
None.
Code Review Checklist: