Skip to content

fix module cache cleanup in Addon._cleanupAddonImports - #20715

Merged
seanbudd merged 2 commits into
nvaccess:masterfrom
LeonarddeR:fixCleanupAddonImports
Aug 26, 2026
Merged

fix module cache cleanup in Addon._cleanupAddonImports#20715
seanbudd merged 2 commits into
nvaccess:masterfrom
LeonarddeR:fixCleanupAddonImports

Conversation

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Link to issue number:

Fixup for #18971.

Summary of the issue:

Installing or removing an add-on can run install tasks. These tasks can import modules from the add-on. Python caches every import in sys.modules. NVDA must drop the add-on's modules from that cache afterwards. Otherwise a later import returns the cached old module instead of the file on disk.

Modules the tasks load directly are tracked in _importedAddonModules and dropped reliably. Modules imported indirectly, for example through a relative import inside the add-on, are not tracked. For those, _cleanupAddonImports scans every module imported during the task and drops each one whose file is inside the add-on directory.

That scan has been broken twice:

Result: updating an add-on can break on the first start after the update. The old version's uninstall task runs first and leaves old modules in the cache. The new version then partly imports old code. A second restart of NVDA resolves the situation regardless.

Description of user facing changes:

Updated add-ons that perform install tasks no longer raise intermittent errors on the first start of NVDA after the update.

Description of developer facing changes:

None.

Description of development approach:

The approach of #15967 was the correct one: a module belongs to the add-on when its file is inside the add-on directory. This change returns to matching on the file path and fixes the three defects of that implementation:

  • The file is read with getattr(module, "__file__", None). Modules without a __file__ are skipped instead of crashing the scan, so the error fixed by Fix bug when updating add-ons automatically #18971 stays fixed.
  • The file path and the add-on directory are compared through os.path.normcase, so case differences match.
  • The directory prefix ends with a path separator, so a sibling directory such as myAddonExtra no longer matches myAddon.

Each defect is covered by a unit test. The loop over the tracked modules is unchanged.

Testing strategy:

New unit tests in tests/unit/test_addonHandler/test_addonImports.py build an add-on in a temporary directory. One of its modules imports a sibling module. Checked:

  • the indirectly imported sibling module is dropped,
  • matching ignores path case,
  • a module without __file__ is kept and nothing raises,
  • a module with __file__ = None is kept,
  • a module from a sibling directory with a shared path prefix is kept,
  • tracked modules are dropped and the record is emptied.

Both historical scans fail this suite. The #18971 form fails the two drop checks. The #15967 form crashes on the missing __file__ check, wrongly drops the sibling-directory module and misses the case difference.

Known issues with pull request:

None.

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.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

cc @nvdaes and @lukaszgo1 because this touches your code.

@LeonarddeR
LeonarddeR force-pushed the fixCleanupAddonImports branch from 662862c to dad8414 Compare August 22, 2026 10:16
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Aug 25, 2026
…ddonImports

_cleanupAddonImports now matches modules on their file path against the
add-on directory, case insensitively and anchored at a path separator.
Modules without a file attribute are skipped.
Add unit tests covering the cleanup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LeonarddeR
LeonarddeR force-pushed the fixCleanupAddonImports branch from dad8414 to e487581 Compare August 25, 2026 06:38
@LeonarddeR
LeonarddeR marked this pull request as ready for review August 25, 2026 06:38
@LeonarddeR
LeonarddeR requested a review from a team as a code owner August 25, 2026 06:38
@LeonarddeR
LeonarddeR requested review from nvdaes and seanbudd and a lite review from Copilot August 25, 2026 06:38

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 add-on module cache cleanup after running install/uninstall tasks, ensuring NVDA reliably purges sys.modules entries that originate from an add-on’s directory (including transitively imported modules). This prevents an updated add-on from accidentally reusing stale, cached modules from a previously installed version.

Changes:

  • Fix Addon._cleanupAddonImports to identify add-on modules via module.__file__ (safely via getattr), with case-normalized path comparison and a directory-boundary-safe prefix.
  • Add a focused unit test suite covering transitive imports, case-insensitive matching, missing/None __file__, sibling-directory false positives, and clearing of tracked imports.
  • Add a user-facing changelog entry describing the fix to add-on updating behavior.

Reviewed changes

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

File Description
user_docs/en/changes.md Adds release note explaining the add-on update reliability fix.
tests/unit/test_addonHandler/test_addonImports.py Adds unit tests to prevent regressions in add-on import cache cleanup.
source/addonHandler/__init__.py Corrects the cleanup scan to match modules by normalized __file__ path within the add-on directory.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread user_docs/en/changes.md Outdated
@seanbudd
seanbudd merged commit fa298b9 into nvaccess:master Aug 26, 2026
43 of 46 checks passed
@github-actions github-actions Bot added this to the 2026.3 milestone Aug 26, 2026
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.

3 participants