Skip to content

Fix app module loading when no process handle can be fetched#18833

Merged
seanbudd merged 8 commits into
nvaccess:masterfrom
LeonarddeR:appModFix
Sep 3, 2025
Merged

Fix app module loading when no process handle can be fetched#18833
seanbudd merged 8 commits into
nvaccess:masterfrom
LeonarddeR:appModFix

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Aug 28, 2025

Copy link
Copy Markdown
Collaborator

Link to issue number:

Fixes #18826 

Summary of the issue:

NVDA sometimes can't get a process handle for an app module reliably.

Description of user facing changes:

Oo some systems, no longer an error on every focus events for some apps, particularly 1Password.

Description of developer facing changes:

NOne

Description of development approach:

When no process handle can be fetched the regular way, enumerate all top level windows until a window is found that belongs to the process, then call oleacc.GetProcessHandleFromHwnd to create a handle.

Testing strategy:

Tested str from #18826 

Known issues with pull request:

Still a bit rough, gathering feedback before continuing

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.
    -x[ ] Security precautions taken.

@SaschaCowley

Copy link
Copy Markdown
Member

@LeonarddeR have you/do you plan to test this with 64 bit builds of NVDA?

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

I can pretty easily test this from x64 source, yes.

@LeonarddeR LeonarddeR marked this pull request as ready for review August 29, 2025 11:35
Copilot AI review requested due to automatic review settings August 29, 2025 11:35
@LeonarddeR LeonarddeR requested a review from a team as a code owner August 29, 2025 11:35
@LeonarddeR LeonarddeR requested a review from seanbudd August 29, 2025 11:35

This comment was marked as outdated.

@LeonarddeR LeonarddeR requested a review from Copilot August 29, 2025 15:49

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 fixes an issue where NVDA was unable to retrieve information for applications when the standard process handle acquisition fails. The fix implements a fallback mechanism that searches for windows belonging to the problematic process and derives a process handle from those windows.

Key changes:

  • Added a fallback mechanism using window enumeration when direct process handle acquisition fails
  • Improved error handling in process handle acquisition functions
  • Enhanced app module loading robustness for applications like 1Password

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
user_docs/en/changes.md Documents the bug fix in the changelog
source/winUser.py Adds utility function for finding top-level windows by predicate
source/winKernel.py Improves error handling in openProcess function
source/winBindings/user32.py Adds Windows API bindings for window enumeration
source/oleacc.py Updates function signature and improves error handling
source/appModuleHandler.py Implements the main fallback logic for process handle acquisition

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread source/winKernel.py
Comment thread source/appModuleHandler.py
Comment thread source/appModuleHandler.py
@LeonarddeR

LeonarddeR commented Aug 29, 2025

Copy link
Copy Markdown
Collaborator Author

I have been able to test this on X64 and it seems to work well for 1password. I'm receiving some debug warnings about csrss.exe though. However, I don't see why NVDA needs an app module for csrss.exe at all.
This app module suffers from the underlying bug reported in #18826, i.e. on beta the process handle is 0, yet isAliveruns true erroneously.

Comment thread source/appModuleHandler.py Outdated
Comment thread source/appModuleHandler.py Outdated
Comment thread source/oleacc.py Outdated
Comment thread user_docs/en/changes.md
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Sep 1, 2025
@seanbudd seanbudd merged commit baa206e into nvaccess:master Sep 3, 2025
5 of 6 checks passed
@github-actions github-actions Bot added this to the 2026.1 milestone Sep 3, 2025
@LeonarddeR LeonarddeR mentioned this pull request Sep 23, 2025
4 tasks
seanbudd pushed a commit that referenced this pull request Sep 24, 2025
Fixes #18859
Related to #18870
Summary of the issue:

Since winBindings and #18833, NVDA spits continuous debug warnings about the inability to get a process handle for csrss.exe
Description of user facing changes:

No errors.
Description of developer facing changes:

None
Description of development approach:

Create a csrss appModule that always assumes csrss.exe to be alive.

    CSRSS is an essential GUI process, therefore it can safely be considered always alive. It hosts the desktop object.
    Before winBindings, the app module was also assumed always alive due to an obfuscated bug.
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.

App module loading severely broken on Alpha

4 participants