Keep NVDA responsive when a UIA or MSAA application stops responding#20168
Conversation
Previously, when an application became unresponsive, synchronous cross-process UIA/MSAA calls into it would block NVDA: the UIA event handlers raised a flood of COMErrors through comtypes (hundreds of identical tracebacks, leaving NVDA partially dead until the app was killed), and the main-thread focus path froze the core for several seconds until the watchdog cancelled the call. UIAHandler: - Add a _catchUIAEventHandlerCOMError decorator on the five UIA event handlers so a COMError from an unresponsive sender is logged once at debug level instead of flooding out through comtypes. - Drop UIA events whose sender's window belongs to an application the system reports as not responding (cheap, cached-handle check). - Use the cached element class name on the high-frequency textChange path instead of a live cross-process fetch (combo box / File Explorer latency). MSAA / main thread: - Drop winEvents for a hung application at the source (beside the existing ghosted-window drop) so the focus ancestor walk (IAccessible accParent) can no longer block the core; engages as soon as the OS flags the app, without waiting for its DWM ghost window. Focus into a hung window is still announced using the hang-safe InternalGetWindowText caption. - Treat IsHungAppWindow like the existing ghost check in isUsableWindow / Window.getPossibleAPIClasses so child windows of a hung app are covered too. Add IsHungAppWindow binding, the UIA.ignoreHungWindowEvents setting (default on) with an Advanced-panel checkbox, unit tests, user guide entry and changelog. Fixes nvaccess#16749 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LeonarddeR
left a comment
There was a problem hiding this comment.
This looks promising really!
Per review feedback from @LeonarddeR and @CyrilleB79: - Remove the _catchUIAEventHandlerCOMError decorator and its uses. Deferred to a follow-up: handlers should ideally not throw, and the gain without it is already large (the hung-window skip prevents the flood in practice). - Remove the "not responding" ui.message announcement from winEventToNVDAEvent. Speaking does not belong in an event-conversion function; a proper announcement can be a separate follow-up. - Drop the UIA.ignoreHungWindowEvents setting (configSpec, Advanced panel checkbox, user guide). It mirrors the existing, unconditional ghost-window handling beside it; there is no legitimate reason to keep polling a window the system reports as not responding, so it does not need to be configurable. - winEventToNVDAEvent: log the dropped hung-window event with log.debugWarning instead of log.debug. The PR is now focused: drop UIA and MSAA events from a not-responding application (beside the existing ghost-window drop) and use the cached class name on the hot textChange path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks @LeonarddeR and @CyrilleB79 — all addressed in the latest push:
The PR is now focused on the core fix: drop UIA and MSAA events from a not-responding application (beside the existing ghost-window drop), and use the cached class name on the hot |
There was a problem hiding this comment.
Pull request overview
This PR aims to keep NVDA responsive when target applications stop responding by detecting “hung” windows and avoiding UI Automation (UIA) / MSAA event processing that can block cross-process calls and flood logs.
Changes:
- Added a User32
IsHungAppWindowbinding and awinUser.isHungAppWindowwrapper. - Dropped UIA events (and switched UIA text-change classification to cached class name) when the sender’s window is detected as hung.
- Dropped MSAA winEvents from hung applications early in
winEventToNVDAEvent; added unit tests for the UIA cached-window-handle guard; updated changelog.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| user_docs/en/changes.md | Documents the user-facing bug fix and UIA text-change responsiveness improvement. |
| tests/unit/test_UIAHandler.py | Adds unit tests for UIA cached window-handle retrieval and the hung-window skip guard. |
| source/winUser.py | Adds isHungAppWindow() wrapper around the new User32 binding. |
| source/winBindings/user32.py | Introduces the IsHungAppWindow ctypes binding. |
| source/UIAHandler/utils.py | Adds helpers to read cached native window handle and decide whether to skip UIA events for hung windows. |
| source/UIAHandler/init.py | Drops UIA events for hung windows; uses cached class name in the textChange path. |
| source/NVDAObjects/window/init.py | Treats hung windows as “unusable” for higher API selection to avoid blocking cross-process calls. |
| source/IAccessibleHandler/init.py | Drops MSAA winEvents for hung windows early to prevent core stalls from cross-process MSAA calls. |
|
Re the @copilot-pull-request-reviewer notes: those reflect the original PR description. Per @LeonarddeR and @CyrilleB79's review the |
|
Thanks @seanbudd, all addressed in the latest commit:
Local ruff and the unit suite are green (7/7). |
- winUser.isHungAppWindow: type hints (hwnd: HWNDVal) -> bool;
L{...} -> backticks in docstring.
- UIAHandler.utils._getCachedWindowHandleFromEvent: epytext @return: ->
sphinx :return:; log the swallowed COMError at debug level.
- UIAHandler.utils._shouldSkipEventForHungWindow: log the swallowed
exception at debug level.
- tests/unit/test_UIAHandler.py: NV Access full copyright header; type
hints on _FakeElement.__init__ and cachedNativeWindowHandle.
- user_docs/en/changes.md: 'UI Automation' -> 'UIA' (per inline suggestion).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks @heath-toby . In future, please don't delete items from the code review checklist. We as reviewers use it to make sure everything is as expected. I've restored the original checklist |
|
This is very interesting work. Thanks. I've looked into various hang scenarios over the years and did most of the watchdog work to prevent (some) hangs. One thing to note here is that Windows can only detect that a window has hung based on unresponsiveness within a timeout period; I think it's 5 seconds. This PR won't prevent a hang in the event that NVDA queries an app after the app has stopped responding, but before Windows detects that it has hung and marks it as such. I see there is further work here to mitigate hangs - #20170, etc. - but I thought this worth noting in case it helps clarify remaining gaps. |
Link to issue number:
Fixes #16749 (also the long-standing #1408).
Summary of the issue:
When an application stops responding, synchronous cross-process UIA/MSAA calls into it block NVDA's core. The UIA event handlers can flood the log with COMError tracebacks and NVDA goes partially unresponsive until the app is killed.
Description of user facing changes:
When an application stops responding, NVDA no longer freezes or floods its log; it stays responsive and stops processing events from the unresponsive application until it recovers.
Description of developer facing changes:
winBindings.user32.IsHungAppWindowctypes binding +winUser.isHungAppWindowwrapper.UIAHandler.utils._getCachedWindowHandleFromEvent/_shouldSkipEventForHungWindowhelpers.Description of development approach:
Beside the existing unconditional ghosted-window drop:
baseCacheRequest, so this read cannot itself block).winEventToNVDAEventdrops MSAA winEvents for a not-responding application (logged vialog.debugWarning).isUsableWindow/Window.getPossibleAPIClassestreat a not-responding window as unusable so NVDA falls back instead of making blocking higher-API calls.textChangepath uses the cached class name instead of a live cross-process fetch.The hung-skip is unconditional, mirroring the adjacent ghosted-window handling — there is no legitimate reason to keep polling a window the system reports as not responding.
Testing strategy:
Unit tests for the cached-window-handle helper and the hung-skip decision. Manual testing against deliberately-hung and real-world hung applications: the COMError flood and the multi-second core freeze no longer occur; NVDA stays responsive and recovers when the application does.
Known issues with pull request:
winEventToNVDAEvent).Code Review Checklist: