Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Daemonize addon store, timeout web requests #16183

Merged
merged 9 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions source/NVDAObjects/behaviors.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,9 @@ def startMonitoring(self):
return
thread = self._monitorThread = threading.Thread(
name=f"{self.__class__.__qualname__}._monitorThread",
target=self._monitor
target=self._monitor,
daemon=True,
)
thread.daemon = True
self._keepMonitoring = True
self._event.clear()
thread.start()
Expand Down
4 changes: 2 additions & 2 deletions source/UIAHandler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,9 @@ def __init__(self):
self.MTAThreadInitException=None
self.MTAThread = threading.Thread(
name=f"{self.__class__.__module__}.{self.__class__.__qualname__}.MTAThread",
target=self.MTAThreadFunc
target=self.MTAThreadFunc,
daemon=True,
)
self.MTAThread.daemon=True
self.MTAThread.start()
self.MTAThreadInitEvent.wait(2)
if self.MTAThreadInitException:
Expand Down
29 changes: 25 additions & 4 deletions source/_addonStore/dataManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@


addonDataManager: Optional["_DataManager"] = None
FETCH_TIMEOUT_S = 120 # seconds


def initialize():
Expand All @@ -67,6 +68,16 @@ def initialize():
addonDataManager = _DataManager()


def terminate():
global addonDataManager
if config.isAppX:
log.info("Add-ons not supported when running as a Windows Store application")
return
addonDataManager.terminate()
log.debug("terminating addonStore data manager")
addonDataManager = None


class _DataManager:
_cacheLatestFilename: str = "_cachedLatestAddons.json"
_cacheCompatibleFilename: str = "_cachedCompatibleAddons.json"
Expand All @@ -89,15 +100,24 @@ def __init__(self):
self._compatibleAddonCache = self._getCachedAddonData(self._cacheCompatibleFile)
self._installedAddonsCache = _InstalledAddonsCache()
# Fetch available add-ons cache early
threading.Thread(
self._initialiseAvailableAddonsThread = threading.Thread(
target=self.getLatestCompatibleAddons,
name="initialiseAvailableAddons",
).start()
daemon=True,
)
self._initialiseAvailableAddonsThread.start()

def terminate(self):
if self._initialiseAvailableAddonsThread.is_alive():
self._initialiseAvailableAddonsThread.join(timeout=1)
if self._initialiseAvailableAddonsThread.is_alive():
log.debugWarning("initialiseAvailableAddons thread did not terminate immediately")

def _getLatestAddonsDataForVersion(self, apiVersion: str) -> Optional[bytes]:
url = _getAddonStoreURL(self._preferredChannel, self._lang, apiVersion)
try:
response = requests.get(url)
log.debug(f"Fetching add-on data from {url}")
response = requests.get(url, timeout=FETCH_TIMEOUT_S)
seanbudd marked this conversation as resolved.
Show resolved Hide resolved
except requests.exceptions.RequestException as e:
log.debugWarning(f"Unable to fetch addon data: {e}")
return None
Expand All @@ -112,7 +132,8 @@ def _getLatestAddonsDataForVersion(self, apiVersion: str) -> Optional[bytes]:
def _getCacheHash(self) -> Optional[str]:
url = _getCacheHashURL()
try:
response = requests.get(url)
log.debug(f"Fetching add-on data from {url}")
response = requests.get(url, timeout=FETCH_TIMEOUT_S)
seanbudd marked this conversation as resolved.
Show resolved Hide resolved
except requests.exceptions.RequestException as e:
log.debugWarning(f"Unable to get cache hash: {e}")
return None
Expand Down
5 changes: 4 additions & 1 deletion source/_addonStore/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,10 @@ def _downloadAddonToPath(
if not NVDAState.shouldWriteToDisk():
return False

with requests.get(addonData.model.URL, stream=True) as r:
# Some add-ons are quite large, so we need to allow for a long download time.
# 1GB at 0.5 MB/s takes 4.5hr to download.
MAX_ADDON_DOWNLOAD_TIME = 60 * 60 * 6 # 6 hours
with requests.get(addonData.model.URL, stream=True, timeout=MAX_ADDON_DOWNLOAD_TIME) as r:
with open(downloadFilePath, 'wb') as fd:
# Most add-ons are small. This value was chosen quite arbitrarily, but with the intention to allow
# interrupting the download. This is particularly important on a slow connection, to provide
Expand Down
2 changes: 1 addition & 1 deletion source/buildVersion.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def formatVersionForGUI(year, major, minor):
name = "NVDA"
version_year = 2023
version_major = 3
version_minor = 3
version_minor = 4
version_build = 0 # Should not be set manually. Set in 'sconscript' provided by 'appVeyor.yml'
version=_formatDevVersionString()
publisher="unknown"
Expand Down
7 changes: 5 additions & 2 deletions source/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ def resetConfiguration(factoryDefaults=False):
hwIo.terminate()
log.debug("terminating addonHandler")
addonHandler.terminate()
# Addons
from _addonStore import dataManager
log.debug("terminating addon dataManager")
dataManager.terminate()
log.debug("Reloading config")
config.conf.reset(factoryDefaults=factoryDefaults)
logHandler.setLogLevelFromConfig()
Expand All @@ -250,8 +254,6 @@ def resetConfiguration(factoryDefaults=False):
lang = config.conf["general"]["language"]
log.debug("setting language to %s"%lang)
languageHandler.setLanguage(lang)
# Addons
from _addonStore import dataManager
dataManager.initialize()
addonHandler.initialize()
# Hardware background i/o
Expand Down Expand Up @@ -856,6 +858,7 @@ def _doPostNvdaStartupAction():
_terminate(bdDetect)
_terminate(hwIo)
_terminate(addonHandler)
_terminate(dataManager, name="addon dataManager")
_terminate(garbageHandler)
# DMP is only started if needed.
# Terminate manually (and let it write to the log if necessary)
Expand Down
7 changes: 6 additions & 1 deletion source/gui/_addonStoreGui/viewModels/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,12 @@ def refresh(self):
_StatusFilterKey.AVAILABLE,
_StatusFilterKey.UPDATE,
}:
threading.Thread(target=self._getAvailableAddonsInBG, name="getAddonData").start()
self._refreshAddonsThread = threading.Thread(
target=self._getAvailableAddonsInBG,
name="getAddonData",
daemon=True,
)
self._refreshAddonsThread.start()

elif self._filteredStatusKey in {
_StatusFilterKey.INSTALLED,
Expand Down
3 changes: 2 additions & 1 deletion source/nvwave.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,8 @@ def play():
fileWavePlayerThread.join()
fileWavePlayerThread = threading.Thread(
name=f"{__name__}.playWaveFile({os.path.basename(fileName)})",
target=play
target=play,
daemon=True,
)
fileWavePlayerThread.start()
else:
Expand Down
4 changes: 2 additions & 2 deletions source/remotePythonConsole.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ def initialize():
server.daemon_threads = True
thread = threading.Thread(
name=__name__, # remotePythonConsole
target=server.serve_forever
target=server.serve_forever,
daemon=True,
)
thread.daemon = True
thread.start()

def terminate():
Expand Down
31 changes: 23 additions & 8 deletions source/updateCheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ def getQualifiedDriverClassNameForStats(cls):
return "%s (core)"%name


UPDATE_FETCH_TIMEOUT_S = 30 # seconds


def checkForUpdate(auto: bool = False) -> Optional[Dict]:
"""Check for an updated version of NVDA.
This will block, so it generally shouldn't be called from the main thread.
Expand Down Expand Up @@ -151,14 +154,16 @@ def checkForUpdate(auto: bool = False) -> Optional[Dict]:
params.update(extraParams)
url = "%s?%s" % (CHECK_URL, urllib.parse.urlencode(params))
try:
res = urllib.request.urlopen(url)
log.debug(f"Fetching update data from {url}")
res = urllib.request.urlopen(url, timeout=UPDATE_FETCH_TIMEOUT_S)
except IOError as e:
if isinstance(e.reason, ssl.SSLCertVerificationError) and e.reason.reason == "CERTIFICATE_VERIFY_FAILED":
# #4803: Windows fetches trusted root certificates on demand.
# Python doesn't trigger this fetch (PythonIssue:20916), so try it ourselves
_updateWindowsRootCertificates()
# and then retry the update check.
res = urllib.request.urlopen(url)
log.debug(f"Fetching update data from {url}")
res = urllib.request.urlopen(url, timeout=UPDATE_FETCH_TIMEOUT_S)
else:
raise
if res.code != 200:
Expand Down Expand Up @@ -253,9 +258,9 @@ def check(self):
"""
t = threading.Thread(
name=f"{self.__class__.__module__}.{self.check.__qualname__}",
target=self._bg
target=self._bg,
daemon=True,
)
t.daemon = True
self._started()
t.start()

Expand Down Expand Up @@ -614,9 +619,9 @@ def start(self):
self._progressDialog.Raise()
t = threading.Thread(
name=f"{self.__class__.__module__}.{self.start.__qualname__}",
target=self._bg
target=self._bg,
daemon=True,
)
t.daemon = True
t.start()

def _guiExec(self, func, *args):
Expand Down Expand Up @@ -656,7 +661,12 @@ def _download(self, url):
# #2352: Some security scanners such as Eset NOD32 HTTP Scanner
# cause huge read delays while downloading.
# Therefore, set a higher timeout.
remote = urllib.request.urlopen(url, timeout=120)
# The NVDA exe is about 35 MB.
# The average download speed in the world is 0.5 MB/s
# in some developing countries with the slowest internet.
# This yields an expected download time of 10min on slower networks.
UPDATE_DOWNLOAD_TIMEOUT = 60 * 30 # 30 min
remote = urllib.request.urlopen(url, timeout=UPDATE_DOWNLOAD_TIMEOUT)
if remote.code != 200:
raise RuntimeError("Download failed with code %d" % remote.code)
size = int(remote.headers["content-length"])
Expand Down Expand Up @@ -851,12 +861,17 @@ class CERT_CHAIN_PARA(ctypes.Structure):
)

def _updateWindowsRootCertificates():
log.debug("Updating Windows root certificates")
crypt = ctypes.windll.crypt32
# Get the server certificate.
sslCont = ssl._create_unverified_context()
# We must specify versionType so the server doesn't return a 404 error and
# thus cause an exception.
u = urllib.request.urlopen(CHECK_URL + "?versionType=stable", context=sslCont)
u = urllib.request.urlopen(
CHECK_URL + "?versionType=stable",
context=sslCont,
timeout=UPDATE_FETCH_TIMEOUT_S,
)
cert = u.fp.raw._sock.getpeercert(True)
u.close()
# Convert to a form usable by Windows.
Expand Down
5 changes: 3 additions & 2 deletions source/virtualBuffers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,9 @@ def loadBuffer(self):
self._loadProgressCallLater = wx.CallLater(1000, self._loadProgress)
threading.Thread(
name=f"{self.__class__.__module__}.{self.loadBuffer.__qualname__}",
target=self._loadBuffer).start(
)
target=self._loadBuffer,
daemon=True,
).start()

def _loadBuffer(self):
try:
Expand Down
4 changes: 2 additions & 2 deletions source/visionEnhancementProviders/NVDAHighlighter.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,10 @@ def __init__(self):
winGDI.gdiPlusInitialize()
self._highlighterThread = threading.Thread(
name=f"{self.__class__.__module__}.{self.__class__.__qualname__}",
target=self._run
target=self._run,
daemon=True,
)
self._highlighterRunningEvent = threading.Event()
self._highlighterThread.daemon = True
self._highlighterThread.start()
# Make sure the highlighter thread doesn't exit early.
waitResult = self._highlighterRunningEvent.wait(0.2)
Expand Down
3 changes: 2 additions & 1 deletion source/watchdog.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ def initialize():
NVDAHelper._setDllFuncPointer(NVDAHelper.localLib, "_notifySendMessageCancelled", _notifySendMessageCancelled)
_watcherThread = threading.Thread(
name=__name__,
target=_watcher
target=_watcher,
daemon=True,
)
alive()
_watcherThread.start()
Expand Down
3 changes: 2 additions & 1 deletion source/winInputHook.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ def initialize():
if hookThreadRefCount==1:
hookThread = threading.Thread(
name=__name__, # winInputHook
target=hookThreadFunc
target=hookThreadFunc,
daemon=True,
)
hookThread.start()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,11 @@ def _start(self):
serve=False # we want to start this serving on another thread so as not to block.
)
log.debug("Server address: {}".format(server.server_address))
server_thread = threading.Thread(target=server.serve, name="RF Test Spy Thread")
server_thread = threading.Thread(
target=server.serve,
name="RF Test Spy Thread",
daemon=True,
)
server_thread.start()

def terminate(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def __init__(self):
self._doSpeechThread = threading.Thread(
target=self._processSpeech,
name="speech spy synth driver",
daemon=True,
)
self._doSpeechThread.start()

Expand Down
8 changes: 8 additions & 0 deletions user_docs/en/changes.t2t
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ What's New in NVDA
%!includeconf: ../changes.t2tconf
%!includeconf: ./locale.t2tconf

= 2023.3.4 =
This is a patch release to fix an installer issue.

== Bug Fixes ==
- Fixed bug which caused the NVDA process to fail to exit correctly.
When running the installer, this resulted in the installation entering an unrecoverable state. (#16122, #16123)
-

= 2023.3.3 =
This is a patch release to fix a security issue.
Please responsibly disclose security issues following NVDA's [security policy https://github.com/nvaccess/nvda/blob/master/security.md].
Expand Down