Skip to content

Keep NVDA responsive when a UIA or MSAA application stops responding#20168

Merged
seanbudd merged 5 commits into
nvaccess:masterfrom
heath-toby:fix/apps-not-responding-crash
May 21, 2026
Merged

Keep NVDA responsive when a UIA or MSAA application stops responding#20168
seanbudd merged 5 commits into
nvaccess:masterfrom
heath-toby:fix/apps-not-responding-crash

Conversation

@heath-toby

@heath-toby heath-toby commented May 18, 2026

Copy link
Copy Markdown
Contributor

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:

  • New winBindings.user32.IsHungAppWindow ctypes binding + winUser.isHungAppWindow wrapper.
  • New UIAHandler.utils._getCachedWindowHandleFromEvent / _shouldSkipEventForHungWindow helpers.

Description of development approach:

Beside the existing unconditional ghosted-window drop:

  • The UIA event handlers skip events whose sender's window belongs to a not-responding application, using only the cached native window handle (NVDA registers event handler groups with baseCacheRequest, so this read cannot itself block).
  • winEventToNVDAEvent drops MSAA winEvents for a not-responding application (logged via log.debugWarning).
  • isUsableWindow / Window.getPossibleAPIClasses treat a not-responding window as unusable so NVDA falls back instead of making blocking higher-API calls.
  • The hot textChange path 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:

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.

heath-toby and others added 2 commits May 18, 2026 18:41
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>
@heath-toby heath-toby marked this pull request as ready for review May 18, 2026 18:25
@heath-toby heath-toby requested review from a team as code owners May 18, 2026 18:25

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks promising really!

Comment thread source/IAccessibleHandler/__init__.py Outdated
Comment thread source/IAccessibleHandler/__init__.py Outdated
Comment thread source/UIAHandler/__init__.py Outdated
Comment thread source/config/configSpec.py Outdated
heath-toby and others added 2 commits May 19, 2026 00:15
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>
@heath-toby

Copy link
Copy Markdown
Contributor Author

Thanks @LeonarddeR and @CyrilleB79 — all addressed in the latest push:

  • Removed the _catchUIAEventHandlerCOMError decorator and its uses (and its unit tests). Agreed it's a safeguard that's better as a follow-up — ideally the handlers shouldn't throw, and the hung-window skip already prevents the flood in practice, so the gain here stands without it.
  • Removed the ui.message "not responding" announcement from winEventToNVDAEvent. Agreed that speaking doesn't belong in an event-conversion function; a proper announcement can be a separate, well-placed change later.
  • Dropped the UIA.ignoreHungWindowEvents setting entirely (configSpec, Advanced-panel checkbox, user guide, tests). It sat right beside the existing unconditional ghost-window drop; per @CyrilleB79's point, there's no legitimate reason to keep polling a window the system reports as not responding, so it doesn't need to be configurable at all. This also avoids the feature-flag question.
  • winEventToNVDAEvent now logs the dropped hung-window event via log.debugWarning (the inline suggestion).

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 textChange path. Local pre-commit, ruff and the unit tests are green.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 IsHungAppWindow binding and a winUser.isHungAppWindow wrapper.
  • 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.

Comment thread source/IAccessibleHandler/__init__.py
Comment thread source/UIAHandler/utils.py
Comment thread source/UIAHandler/__init__.py
@heath-toby

Copy link
Copy Markdown
Contributor Author

Re the @copilot-pull-request-reviewer notes: those reflect the original PR description. Per @LeonarddeR and @CyrilleB79's review the _catchUIAEventHandlerCOMError decorator, the UIA.ignoreHungWindowEvents config option, and the "not responding" ui.message announcement were intentionally removed/deferred, and the code was scoped down accordingly. I've now updated the PR description to match the actual implementation. Dropping focus events for a not-responding window (and a top-level COMError safeguard) are deliberate follow-ups, not omissions.

Comment thread source/UIAHandler/utils.py Outdated
Comment thread source/winUser.py Outdated
Comment thread source/winUser.py Outdated
Comment thread source/UIAHandler/utils.py
Comment thread source/UIAHandler/utils.py
Comment thread tests/unit/test_UIAHandler.py
Comment thread tests/unit/test_UIAHandler.py Outdated
Comment thread tests/unit/test_UIAHandler.py Outdated
Comment thread user_docs/en/changes.md Outdated
@seanbudd seanbudd marked this pull request as draft May 19, 2026 02:51
@heath-toby

Copy link
Copy Markdown
Contributor Author

Thanks @seanbudd, all addressed in the latest commit:

  • Type hints on isHungAppWindow (hwnd: HWNDVal) -> bool) and on _FakeElement.__init__ / cachedNativeWindowHandle.
  • Docstring tags modernised on the two lines you flagged (@return: -> :return:, L{...} -> backticks).
  • _getCachedWindowHandleFromEvent now log.debugs the swallowed COMError; _shouldSkipEventForHungWindow likewise log.debugs the swallowed broad exception (with exc_info).
  • Test file uses the NV Access full copyright header per the linked guide.
  • Changelog: applied your "UIA" wording suggestion verbatim.

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>
@seanbudd

Copy link
Copy Markdown
Member

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

@seanbudd seanbudd marked this pull request as ready for review May 20, 2026 23:41

@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 @heath-toby

@seanbudd seanbudd merged commit e5abefc into nvaccess:master May 21, 2026
39 of 42 checks passed
@github-actions github-actions Bot added this to the 2026.3 milestone May 21, 2026
@jcsteh

jcsteh commented May 21, 2026

Copy link
Copy Markdown
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nvda hangs when ui freezes

6 participants