Skip to content

Fixes to address hangs caused by WASAPI idle checks#15349

Merged
seanbudd merged 2 commits into
nvaccess:masterfrom
jcsteh:wasapiHang
Aug 31, 2023
Merged

Fixes to address hangs caused by WASAPI idle checks#15349
seanbudd merged 2 commits into
nvaccess:masterfrom
jcsteh:wasapiHang

Conversation

@jcsteh

@jcsteh jcsteh commented Aug 31, 2023

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #15150.

Summary of the issue:

When WASAPI is enabled, NVDA sometimes hangs and cannot be recovered, especially when switching audio devices. The freezes all occur in WasapiWavePlayer._idleCheck when calling wasPlay_idle.

Description of user facing changes

Fixed the hangs.

Description of development approach

  1. If getting the playback position fails with an error, playback has probably been interrupted; e.g. because the device disconnected. Treat this as if playback has finished so we don't wait forever and so that we fire any pending callbacks.
  2. Never treat a stream as idle while we're feeding audio to it. Aside from this not making any sense, we might otherwise try to sync and stop it in the main thread, which could cause hangs or crashes.

Testing strategy:

I'm unable to reproduce this, so I asked several people to test try builds in #15150. They report that this addresses the problem for them.
I also verified (using beeps in the code, now removed) that streams are still correctly treated as idle when they should be.

Known issues with pull request:

None.

Change log entries:

Bug fixes
When WASAPI is enabled, NVDA no longer hangs occasionally, especially when switching audio devices.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@jcsteh jcsteh requested a review from a team as a code owner August 31, 2023 03:37
@jcsteh jcsteh requested a review from seanbudd August 31, 2023 03:37

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jcsteh .

I'm not going to use the change log entry as this fixes an unreleased issue (kinda)

@seanbudd seanbudd merged commit fb82d9b into nvaccess:master Aug 31, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.3 milestone Aug 31, 2023
@jcsteh jcsteh deleted the wasapiHang branch May 25, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intermitent crashing on latest alphas

3 participants