Skip to content

Improve typing and handling of updateCheck state#14739

Merged
seanbudd merged 2 commits into
masterfrom
improveStateChecking
Mar 24, 2023
Merged

Improve typing and handling of updateCheck state#14739
seanbudd merged 2 commits into
masterfrom
improveStateChecking

Conversation

@seanbudd

@seanbudd seanbudd commented Mar 23, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Link to issue number:

Reported privately

Summary of the issue:

If the updateCheckState pickle is set to "None", the state fails to load correctly.
This can cause a crash.
It is uncertain as to the cause of the pickle being set to None, but it is assumed to do with exiting NVDA during initialization.

initializing updateCheck
CRITICAL - __main__ (08:40:58.427) - MainThread (8816):
core failure
Traceback (most recent call last):
  File "nvda.pyw", line 427, in <module>
  File "core.pyc", line 737, in main
  File "updateCheck.pyc", line 810, in initialize
TypeError: argument of type 'NoneType' is not iterable

nvda/source/updateCheck.py

Lines 792 to 813 in af0e998

def initialize():
global state, _stateFilename, autoChecker
_stateFilename = os.path.join(globalVars.appArgs.configPath, "updateCheckState.pickle")
try:
# #9038: Python 3 requires binary format when working with pickles.
with open(_stateFilename, "rb") as f:
state = pickle.load(f)
except:
log.debugWarning("Couldn't retrieve update state", exc_info=True)
# Defaults.
state = {
"lastCheck": 0,
"dontRemindVersion": None,
}
_setStateToNone(state)
# check the pending version against the current version
# and make sure that pendingUpdateFile and pendingUpdateVersion are part of the state dictionary.
if "pendingUpdateVersion" not in state or state["pendingUpdateVersion"] == versionInfo.version:
_setStateToNone(state)
# remove all update files except the one that is currently pending (if any)
try:

Description of user facing changes

Should fix rare crash where updateCheck.pickle is written to with invalid state information (unknown cause).

Description of development approach

Add typing, handle the case where pickle.load loads None for the state.

Testing strategy:

  1. Set %appdata%/nvda/updateCheckState.pickle to "N." which loads as "None"
  2. Test if NVDA crashes on start up

Known issues with pull request:

None

Change log entries:

Not needed, rare bug

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@seanbudd seanbudd marked this pull request as ready for review March 23, 2023 23:39
@seanbudd seanbudd requested a review from a team as a code owner March 23, 2023 23:39
@seanbudd seanbudd requested a review from michaelDCurran March 23, 2023 23:39
@AppVeyorBot

AppVeyorBot commented Mar 24, 2023

Copy link
Copy Markdown

See test results for failed build of commit b1c1ef4fd1

@seanbudd seanbudd merged commit 42dbabc into master Mar 24, 2023
@seanbudd seanbudd deleted the improveStateChecking branch March 24, 2023 01:29
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelDCurran michaelDCurran michaelDCurran approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2023.2

Development

Successfully merging this pull request may close these issues.

4 participants

@seanbudd @AppVeyorBot @michaelDCurran @nvaccessAuto