Rate limiting against gui crash

[seanbudd]

Link to issue number:

Blocked by #19702
Fixes #19634

Summary of the issue:

Windows throws errors when rapidly requesting a window handle on the secure desktop.
This cause visual tearing and a crash of NVDA.

Description of user facing changes:

Avoid crash and tearing of controls

Description of developer facing changes:

Add custom handling of changing settings categories for secure mode.

Description of development approach:

This pull request introduces a debounced category change mechanism in the NVDASettingsDialog to improve stability and user experience, particularly when running on a secure desktop.

• Added a debounce mechanism (50ms delay) for handling category changes in NVDASettingsDialog when running on a secure desktop, using a timer to prevent rapid or duplicate category switches. This helps avoid potential issues with UI responsiveness or event handling in secure desktop environments. [1] [2]
• Introduced the use of isRunningOnSecureDesktop from utils.security to determine when to apply the debounce logic.
• Added type annotations to the onCategoryChange method and new internal variables for better code clarity and maintainability. [1] [2]
• Ensured proper cleanup of the debounce timer and related state in the Destroy method to prevent resource leaks.

An alternative approach with error suppression was tried, but visual tearing still occurred.
https://github.com/nvaccess/nvda/compare/try-gui-crash-2?expand=1

Testing strategy:

Tested STR in #19634

Known issues with pull request:

Other instances may exist of rapidly requesting a window handle.
Should this be reported to wxWidgets or microsoft?
Should we create a generic monkey patch or something?
From basic investigating, the only real way to cause this could be through updating controls in the settings dialog, or the config profiles dialog.

Code Review Checklist:

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

[seanbudd]

Hi @CyrilleB79 - could you test with this try build?

[Copilot]

Pull request overview

This PR addresses a secure-desktop-only NVDA settings dialog instability/crash triggered by rapid category switching (issue #19634). It adds a small debounce layer around category changes when NVDA is running on the secure desktop to reduce rapid UI rebuilds that can lead to control tearing and crashes.

Changes:

• Added a 50ms debounce for settings category changes on the secure desktop via wx.CallLater.
• Introduced secure-desktop detection (isRunningOnSecureDesktop) to conditionally apply the debounce logic.
• Ensured debounce timer/state is cleaned up when the settings dialog is destroyed.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[CyrilleB79]

I won't probably not be able to test before the end of this week. Though, since the PR is in Draft state, do I still need to test it now or should I wait for #19702?

[seanbudd]

Either way is appreciated. Testing now confirms that debouncing fixes the problem, testing later confirms our next debouncing algorithm works still

[CyrilleB79]

Hi.

I have just tested nvda_snapshot_try-gui-crash-54816,4ece670b.exe, with service debug key enabled so that I can take a log from the secure screen.

Unfortunately, the issue is still reproducible switching between categories or maintaining the tab key pressed to move the focus around controls. Here is the log: nvda.log

I have used ctrl+alt+suppr screen from my session to experiment.

[seanbudd]

Is this something you can reproduce without the serviceDebug key? I think there might be a separate issue with certain controls showing while in serviceDebug mode when they shouldn't

[CyrilleB79]

Is this something you can reproduce without the serviceDebug key?

I'd say yes, but let me double check Tonight (Europe time) to be sure. I'll let you know.

[CyrilleB79]

Is this something you can reproduce without the serviceDebug key?

Just tested now. Unfortunately the issue is still present without the service debug key. Meybe a bit more difficult to reproduce; it's a bit hard to say since the issue occurs only rarely except when I perform stress tests.

[seanbudd]

I've found I could also reproduce still, but less frequently. I've upped the debounce time significantly for testing again

[seanbudd]

I can't reproduce with the very high debouce limit.
Can you confirm with testing @CyrilleB79 - try build?
I think we should merge with the higher limit, and reduce it on alphas if need be.

[CyrilleB79]

OK, I'll test tonight (CET)

[CyrilleB79]

I have just tested nvda_snapshot_try-gui-crash-54962,3ca5270b.exe.

Unfortunately, the issue is still present. I can still reproduce GUI controls/panels mixing (3 times) after about 30 seconds of stress tests with tab, shift+tab, ctrl+tab and ctrl+shift+tab to navigate between controls and panels.

[seanbudd]

Thanks for testing @CyrilleB79. I think we are going to keep the current limits for now, since they seem to work for the average user. I think the tradeoff between usability and preventing the crash is best at around 500ms. I hope beyond stress testing, that should be enough to prevent a crash for the average user?