Skip to content

NVDA no longer becomes unusable when interacting with a protected process such as 1Password#18948

Merged
seanbudd merged 3 commits into
masterfrom
accessDenied
Sep 19, 2025
Merged

NVDA no longer becomes unusable when interacting with a protected process such as 1Password#18948
seanbudd merged 3 commits into
masterfrom
accessDenied

Conversation

@michaelDCurran
Copy link
Copy Markdown
Member

Link to issue number:

None

Summary of the issue:

During the conversion to 64 bit, the ctypes definition of WaitForSingleObject was tightened up. This caused related errors in NVDA previously hidden to be surfaced, such as continuing to keep an appModule alive with an invalid process handle. this was addressed.
However, a similar issue was also surfaced where although we successfully open a process, asking for the SYNCHRONIZE access right, once we go to check if the process is dead by calling WaitforSingleObject(handle, 0) this fails with access denied. We never actually got the SYNCHRONIZE access right.
This is most serious when trying to interact with 1Password, where it becomes totally unusable and NVDA continuously torws the error:

  File "appModuleHandler.pyc", line 622, in _get_isAlive
  File "winKernel.pyc", line 344, in waitForSingleObject
PermissionError: [WinError 5] Access is denied.

This error is also randomly seen from time to time when alt tabbing between windows. There may be another protected Windows process which happens to be noticed by NVDA.

Description of user facing changes:

NVDA no longer becomes unusable when interacting with a protected process.

Description of developer facing changes:

Description of development approach:

AppModule.isAlive: specifically handle the case when WaitForSingleObject fails, and the related error is ERROR_ACCESS_DENIED. Log a debugWarning the first time, but then set a special _liveForEver instance variable on the AppModule to true, communicating to isAlive that it should no longer check. We are not going to magically get access, so all we can do is assume it continues to be alive.

The debugWarning looks like:

Access denied waiting on Process handle 2832 for AppModule(1password, appName='1password', processID=20012), cannot verify dead, marking as living for ever.

Testing strategy:

  • Open 1Password and interact with it. NVDA no longer throws errors making 1Password and NVDA unusable.
  • Use NvDA for an extended period to confirm the random PermissionError exceptions have disappeared.

Known issues with pull request:

AppModules for Protected processes will stay around for the lifetime of NvDA. In 1Password's case, its process seems to stay around a lot anyway. And any other protected process probably has no specific appModule.

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.

…h access denied, as the process living for ever, avoiding the endless PermissionError exceptions making NVDA unusable. Processes such as 1Password disallow waiting on the process.
Copilot AI review requested due to automatic review settings September 19, 2025 03:45
@michaelDCurran michaelDCurran requested a review from a team as a code owner September 19, 2025 03:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 NVDA becoming unusable when interacting with protected processes like 1Password by properly handling access denied errors when checking process liveness.

  • Adds graceful handling of ERROR_ACCESS_DENIED when waiting on process handles
  • Implements a _liveForEver flag to mark protected processes as permanently alive
  • Logs debug warnings for access denied scenarios instead of crashing

Reviewed Changes

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

File Description
source/winKernel.py Adds ERROR_ACCESS_DENIED constant for error handling
source/appModuleHandler.py Implements _liveForEver flag and error handling for protected processes

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

Comment thread source/appModuleHandler.py Outdated
Comment thread source/appModuleHandler.py Outdated
Comment thread source/appModuleHandler.py Outdated
pre-commit-ci Bot and others added 2 commits September 19, 2025 03:46
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread source/winKernel.py
@michaelDCurran
Copy link
Copy Markdown
Member Author

michaelDCurran commented Sep 19, 2025 via email

@seanbudd seanbudd merged commit 0bbc271 into master Sep 19, 2025
40 checks passed
@seanbudd seanbudd deleted the accessDenied branch September 19, 2025 04:49
@github-actions github-actions Bot added this to the 2026.1 milestone Sep 19, 2025
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.

4 participants