Skip to content

Commit

Permalink
Downgrade to Python 3.7 due to stack corruption in Python 3.8+ (nvacc…
Browse files Browse the repository at this point in the history
…ess#12298)

Fixes nvaccess#12152
Fixes nvaccess#12154

Since upgrading to Python 3.8, several serious crashes in NVDA have been reported. Specifically:
• NVDA crashing when using the SAPI4 speech synthesizer: nvaccess#12152
• NVDA crashing when using Windows Explorer on Windows Server 2012: nvaccess#12154
Both of these issues were traced to stack corruption after a Python callback of NVDA's was called from external libraries. In SAPI4's case, after calling NVDA's implementation of ITTSBufNotifySink::TextDataStarted, and in the Windows Server 2012 case: IUIAutomationPropertyChangedEventHandler::handlePropertyChangedEvent.
It seems as though libFFI / Python ctypes is not cleaning the stack properly after executing a Python callback with the stdcall calling convention (ctypes WINFUNCTYPE), where the callback contained at least one argument larger than 4 bytes (E.g. a long long, or a VARIANT struct), and the arguments preceding it were such that this argument was not aligned to an 8 byte boundary. E.g. the callback might be:
• callback(void*, long long) or
• callback(void*, void*, int, VARIANT)
See Python bug: https://bugs.python.org/issue38748
On that bug I have attached a minimal testcase.
This bug affects both Python 3.8 and Python 3.9.
The bug is most likely in the libFFI project used by Python's ctypes module. Python 3.8 switched to a much more recent and official version of libFFI I believe.
Although we do really want to move to Python 3.8+ as soon as we can, right now this bug makes it impossible to do so. We could specifically work around the currently known manifestations by moving some of that code into C++, but that brings its own risks, and we still don't know where else this issue may appear in our code. The appropriate thing to do right now is stay on Python 3.7 until we can work with Python / libFFI to get this resolved.

Description of how this pull request fixes the issue:
Downgrades to Python 3.7 by referencing Python 3.7 (rather than 3.8) in NVDA's build system.
The existing PRs that needed to be reverted were:
• Updating brlAPI to a Python 3.8 build: nvaccess/nvda-misc-deps#20
• Switching to using Python's own pgettext: nvaccess#12109
• calling os.add_dll_directory when loading liblouis: nvaccess#12020
None of these PRs provided any user visible changes.
The rest of the Python 3.8 work, including the switch to a virtual
environment etc is all compatible with Python 3.7 and can remain.
  • Loading branch information
michaelDCurran authored Apr 20, 2021
1 parent ce28256 commit e019a24
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 22 deletions.
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ branches:
- /release-.*/

environment:
PY_PYTHON: 3.8-32
PY_PYTHON: 3.7-32
encFileKey:
secure: ekOvuyywHuDdGZmRmoj+b3jfrq39A2xlx4RD5ZUGd/8=
mozillaSymsAuthToken:
Expand Down
2 changes: 1 addition & 1 deletion miscDeps
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ The NVDA source depends on several other packages to run correctly.
### Installed Dependencies
The following dependencies need to be installed on your system:

* [Python](https://www.python.org/), version 3.8, 32 bit
* [Python](https://www.python.org/), version 3.7, 32 bit
* Use latest minor version if possible.
* Microsoft Visual Studio 2019 Community, Version 16.3 or later:
* Download from https://visualstudio.microsoft.com/vs/
Expand Down
2 changes: 1 addition & 1 deletion sconstruct
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if nvdaVenv != virtualEnv:

# Variables for storing required version of Python, and the version which is used to run this script.
requiredPythonMajor ="3"
requiredPythonMinor = "8"
requiredPythonMinor = "7"
requiredPythonArchitecture = "32bit"
installedPythonMajor = str(sys.version_info.major)
installedPythonMinor = str(sys.version_info.minor)
Expand Down
7 changes: 4 additions & 3 deletions source/addonHandler/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: UTF-8 -*-
# addonHandler.py
# A part of NonVisual Desktop Access (NVDA)
# Copyright (C) 2012-2021 NV Access Limited, Rui Batista, Noelia Ruiz Martínez,
# Copyright (C) 2012-2019 Rui Batista, NV Access Limited, Noelia Ruiz Martínez,
# Joseph Lee, Babbage B.V., Arnold Loubriat
# This file is covered by the GNU General Public License.
# See the file COPYING for more details.
Expand Down Expand Up @@ -539,8 +540,8 @@ def initTranslation():
try:
callerFrame = inspect.currentframe().f_back
callerFrame.f_globals['_'] = translations.gettext
# Install pgettext function.
callerFrame.f_globals['pgettext'] = translations.pgettext
# Install our pgettext function.
callerFrame.f_globals['pgettext'] = languageHandler.makePgettext(translations)
finally:
del callerFrame # Avoid reference problems with frames (per python docs)

Expand Down
32 changes: 28 additions & 4 deletions source/languageHandler.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
# languageHandler.py
# A part of NonVisual Desktop Access (NVDA)
# Copyright (C) 2007-2021 NV Access Limited, Joseph Lee
# Copyright (C) 2007-2018 NV access Limited, Joseph Lee
# This file is covered by the GNU General Public License.
# See the file COPYING for more details.

"""Language and localization support.
This module assists in NVDA going global through language services such as converting Windows locale ID's to friendly names and presenting available languages.
"""

import builtins
import os
import sys
import ctypes
Expand Down Expand Up @@ -124,6 +126,28 @@ def getAvailableLanguages(presentational=False):
)
return langs


def makePgettext(translations):
"""Obtaina pgettext function for use with a gettext translations instance.
pgettext is used to support message contexts,
but Python's gettext module doesn't support this,
so NVDA must provide its own implementation.
"""
if isinstance(translations, gettext.GNUTranslations):
def pgettext(context, message):
try:
# Look up the message with its context.
return translations._catalog[u"%s\x04%s" % (context, message)]
except KeyError:
return message
elif isinstance(translations, gettext.NullTranslations):
# A language with out a translation catalog, such as English.
def pgettext(context, message):
return message
else:
raise ValueError("%s is Not a GNUTranslations or NullTranslations object" % translations)
return pgettext

def getWindowsLanguage():
"""
Fetches the locale name of the user's configured language in Windows.
Expand Down Expand Up @@ -178,10 +202,10 @@ def setLanguage(lang: str) -> None:
trans = gettext.translation("nvda", fallback=True)
curLang = "en"

# #9207: Python 3.8 adds gettext.pgettext, so add it to the built-in namespace.
trans.install(names=["pgettext"])
trans.install()
setLocale(curLang)

# Install our pgettext function.
builtins.pgettext = makePgettext(trans)

def setLocale(localeName: str) -> None:
'''
Expand Down
10 changes: 3 additions & 7 deletions source/louisHelper.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
# louisHelper.py
# A part of NonVisual Desktop Access (NVDA)
# This file is covered by the GNU General Public License.
# See the file COPYING for more details.
# Copyright (C) 2018-2021 NV Access Limited, Babbage B.V., Joseph Lee
# Copyright (C) 2018 NV Access Limited, Babbage B.V.

"""Helper module to ease communication to and from liblouis."""

# Python 3.8 changes the way DLL's are loaded due to security.
# Thus manually add NVDA executable path to DLL lookup path for loading liblouis.dll.
import os
import globalVars
with os.add_dll_directory(globalVars.appDir):
import louis
import louis
from logHandler import log
import config

Expand Down
1 change: 0 additions & 1 deletion user_docs/en/changes.t2t
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ What's New in NVDA

== Changes for Developers ==
- Note: this is a Add-on API compatibility breaking release. Add-ons will need to be re-tested and have their manifest updated.
- NVDA now requires Python 3.8. (#12075)
- NVDA's build system now fetches all Python dependencies with pip and stores them in a Python virtual environment. This is all done transparently.
- To build NVDA, SCons should continue to be used in the usual way. E.g. executing scons.bat in the root of the repository. Running ``py -m SCons`` is no longer supported, and ``scons.py`` has also been removed.
- To run NVDA from source, rather than executing ``source/nvda.pyw`` directly, the developer should now use ``runnvda.bat`` in the root of the repository. If you do try to execute ``source/nvda.pyw``, a message box will alert you this is no longer supported.
Expand Down
4 changes: 2 additions & 2 deletions user_docs/en/userGuide.t2t
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ For details regarding exceptions, access the license document from the NVDA menu

+ System Requirements +[SystemRequirements]
- Operating Systems: all 32-bit and 64-bit editions of Windows 7, Windows 8, Windows 8.1, Windows 10, and all Server Operating Systems starting from Windows Server 2008 R2.
- For Windows 7 and Windows Server 2008 R2 NVDA requires Service Pack 1 and the [KB3063858 https://support.microsoft.com/en-us/kb/3063858] update.
Both of these should be installed if all available updates are applied.
- For Windows 7, NVDA requires Service Pack 1 or higher.
- For Windows Server 2008 R2, NVDA requires Service Pack 1 or higher.
- at least 150 MB of storage space.
-

Expand Down
2 changes: 1 addition & 1 deletion venvUtils/ensureAndActivate.bat
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ rem and then activates it.
rem This is an internal script and should not be used directly.

rem Ensure the environment is created and up to date
py -3.8-32 "%~dp0\ensureVenv.py"
py -3.7-32 "%~dp0\ensureVenv.py"
if ERRORLEVEL 1 goto :EOF

rem Set the necessary environment variables to have Python use this virtual environment.
Expand Down
11 changes: 11 additions & 0 deletions venvUtils/ensureVenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,15 @@ def ensureVenvAndRequirements():
"Please deactivate the current Python virtual environment and try again."
)
sys.exit(1)
if (
sys.version_info.minor == 7
and sys.version_info.micro == 6
):
# #10696: Building with Python 3.7.6 fails. Inform user and exit.
Py376FailMsg = (
"Error: Building with Python 3.7.6 is not possible.\n"
"Please use a more recent version of Python 3."
)
print(Py376FailMsg)
sys.exit(1)
ensureVenvAndRequirements()

0 comments on commit e019a24

Please sign in to comment.