Python 3.8/liblouis helper: add NVDA executable path by calling os.add_dll_directory #12020

josephsl · 2021-01-31T21:24:25Z

Hi,

Required in Python 3.8:

Link to issue number:

Summary of the issue:

Python 3.8 adds os.add_dll_directory function to add a specified path to known DLL folders, used to load DLL's more securely. Without this call, liblouis.dll cannot be imported - instead, Python says "ModuleNotFoundError".

Description of how this pull request fixes the issue:

Calls os.add_dll_directory function (context manager form) from louisHelper.py to add NVDA executable path to known DLL's directory in order to import liblouis.dll.

Testing performed:

Tested from source code version of Python 3.8 NVDA with no problems - braille input and output works as expected.

Known issues with pull request:

None

Change log entry:

None

Thanks.

AppVeyorBot · 2021-01-31T21:34:24Z

Unit test failure
Build (for testing PR)

See test results for failed build of commit 184538c371

josephsl · 2021-01-31T21:37:10Z

Hi,

No need to worry about Nose test failure.

Thanks.

…luis DLL. Python 3.8 adds os.add_dll_directory to securely load DLL's from known locations, which affects Louis helper when trying to load liblouis.dll from NVDA executable folder. Therefore add NVDA executable path to known DLL directories through os.add_dll_directory, which resolves ModuleNotFoundError.

AppVeyorBot · 2021-02-05T19:00:37Z

Unit test failure
Build (for testing PR)

See test results for failed build of commit c3957f5396

lukaszgo1 · 2021-02-06T15:29:18Z

source/louisHelper.py

-#This file is covered by the GNU General Public License.
-#See the file COPYING for more details.
-#Copyright (C) 2018 NV Access Limited, Babbage B.V.
+# louisHelper.py


Please remove this line

lukaszgo1 · 2021-02-06T15:31:20Z

source/louisHelper.py

+# 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
+with os.add_dll_directory(os.path.dirname(__file__)):


os.path.dirname(__file__) is not gonna work for binary copies since it would point to nvda_install_dir\library.zip. Please use globalVars.appDir instead.

josephsl · 2021-02-06T17:45:32Z

Hi, forgot about globalVars.appDir. Thanks for pointing this out.

…ble path. Reviewed by Lukasz Golonka: remove file name from header comment. Also, because __file__ will point to library.zip, use globalVars.appDir to locate NVDA executable where liblouis.dll actually resides.

josephsl · 2021-02-12T18:58:20Z

Hi,

Upon further testing, globalVars.appDir will not work on source code version but file will. I'll find out if the binary copy is affected by this and come up with something if needed.

Thanks.

josephsl · 2021-02-12T19:28:26Z

Hi,

Traceback from source version while using globalVars.appDir:

CRITICAL - main (10:56:05.868) - MainThread (13492):
core failure
Traceback (most recent call last):
File "nvda.pyw", line 247, in
core.main()
File "core.py", line 268, in main
import appModuleHandler
File "appModuleHandler.py", line 27, in
import NVDAHelper
File "NVDAHelper.py", line 26, in
import eventHandler
File "eventHandler.py", line 12, in
import api
File "api.py", line 12, in
import review
File "review.py", line 10, in
from NVDAObjects import NVDAObject, NVDAObjectTextInfo
File "NVDAObjects_init_.py", line 18, in
from displayModel import DisplayModelTextInfo
File "displayModel.py", line 17, in
import mouseHandler
File "mouseHandler.py", line 8, in
import gui
File "gui_init_.py", line 25, in
import ui
File "ui.py", line 22, in
import braille
File "braille.py", line 20, in
import louisHelper
File "louisHelper.py", line 13, in
import louis
File "louis_init_.py", line 61, in
liblouis = loader["liblouis.dll"]
File "C:\Python38\lib\ctypes_init.py", line 448, in getitem
return getattr(self, name)
File "C:\Python38\lib\ctypes_init_.py", line 443, in getattr
dll = self.dlltype(name)
File "C:\Python38\lib\ctypes_init.py", line 373, in init
self._handle = _dlopen(self._name, mode)
FileNotFoundError: Could not find module 'liblouis.dll' (or one of its dependencies). Try using the full path with constructor syntax.

This was after compiling NVDA from source (Python 3.8/SCons).

Thanks.

lukaszgo1 · 2021-02-12T19:39:46Z

That's quite strange. Have you compared os.path.dirname(__file__) with globalVars.appDir logically they should point to the same location?

josephsl · 2021-02-12T19:46:56Z

Hi, nope it doesn’t. As you pointed out, using __file__ results in library.zip whereas globalVars.appDir results in the parent folder of the executable file e.g. if code is located in nvdapy38/source, globalVars.appDir will point to just nvdapy38 folder. Although ugly, one possible workaround is detecting source vs binary and add “source” to source code version. What’s more interesting is that liblouis.dll is loaded as part of braille input/output initialization later (and one can then work with the DLL from that point on), as the issue with louisHelper/add_dll_directory is encountered early in core.main. Thanks.

lukaszgo1 · 2021-02-13T12:25:39Z

source/louisHelper.py

+# Thus manually add NVDA executable path to DLL lookup path for loading liblouis.dll.
+import os
+import globalVars
+with os.add_dll_directory(os.path.dirname(globalVars.appDir)):


I believe I've found the problem. There is no point in using (os.path.dirname on globalVars.appDir as it represents path to the directory already. On my machine using globalVars.appDir alone allows source copy of NVDA to import liblouis and start correctly whereas when (os.path.dirname is used I'm getting the traceback you've posted yesterday.

…h os.path.dirname. Pointed out by Lukasz Golonka: globalVars.appDir records the actual path to the executable, including in source code version (nvda.pyw). Therefore use app dir directly.

…ng os.add_dll_directory (#12020)" This reverts commit 5795e76.

Fixes #12152 Fixes #12154 Since upgrading to Python 3.8, several serious crashes in NVDA have been reported. Specifically: • NVDA crashing when using the SAPI4 speech synthesizer: #12152 • NVDA crashing when using Windows Explorer on Windows Server 2012: #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: #12109 • calling os.add_dll_directory when loading liblouis: #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.

josephsl added the Python 3.8 Issues or ideas that can be resolved or derive solutions via Python 3.8. label Jan 31, 2021

josephsl added 2 commits February 5, 2021 10:50

Louis helper: update copyright header

af482e1

josephsl force-pushed the py38_addDLLDirectory branch from c3aba71 to 714db9c Compare February 5, 2021 18:50

josephsl changed the base branch from master to py3.8 February 5, 2021 18:56

lukaszgo1 suggested changes Feb 6, 2021

View reviewed changes

Louis helper: remove header comment, use globalVars.appDir as executa…

129c847

…ble path. Reviewed by Lukasz Golonka: remove file name from header comment. Also, because __file__ will point to library.zip, use globalVars.appDir to locate NVDA executable where liblouis.dll actually resides.

lukaszgo1 reviewed Feb 13, 2021

View reviewed changes

Louis helper: use globalVars.appDir directly rather than going throug…

e9fbce0

…h os.path.dirname. Pointed out by Lukasz Golonka: globalVars.appDir records the actual path to the executable, including in source code version (nvda.pyw). Therefore use app dir directly.

michaelDCurran approved these changes Feb 16, 2021

View reviewed changes

michaelDCurran merged commit 5795e76 into nvaccess:py3.8 Feb 16, 2021

nvaccessAuto added this to the 2021.1 milestone Feb 16, 2021

Adriani90 mentioned this pull request Feb 16, 2021

Python 3.8: add NVDA source directory when attempting to import liblouis DLL #10416

Closed

josephsl deleted the py38_addDLLDirectory branch March 1, 2021 01:27

michaelDCurran added a commit that referenced this pull request Apr 15, 2021

Revert "Python 3.8/liblouis helper: add NVDA executable path by calli…

ea66bbc

…ng os.add_dll_directory (#12020)" This reverts commit 5795e76.

michaelDCurran mentioned this pull request Apr 16, 2021

Downgrade to Python 3.7 due to stack corruption in Python 3.8+ #12298

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3.8/liblouis helper: add NVDA executable path by calling os.add_dll_directory #12020

Python 3.8/liblouis helper: add NVDA executable path by calling os.add_dll_directory #12020

josephsl commented Jan 31, 2021 •

edited

Loading

AppVeyorBot commented Jan 31, 2021

josephsl commented Jan 31, 2021

AppVeyorBot commented Feb 5, 2021

lukaszgo1 Feb 6, 2021

lukaszgo1 Feb 6, 2021

josephsl commented Feb 6, 2021 via email

josephsl commented Feb 12, 2021

josephsl commented Feb 12, 2021

lukaszgo1 commented Feb 12, 2021

josephsl commented Feb 12, 2021 via email

lukaszgo1 Feb 13, 2021

Python 3.8/liblouis helper: add NVDA executable path by calling os.add_dll_directory #12020

Python 3.8/liblouis helper: add NVDA executable path by calling os.add_dll_directory #12020

Conversation

josephsl commented Jan 31, 2021 • edited Loading

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

AppVeyorBot commented Jan 31, 2021

josephsl commented Jan 31, 2021

AppVeyorBot commented Feb 5, 2021

lukaszgo1 Feb 6, 2021

Choose a reason for hiding this comment

lukaszgo1 Feb 6, 2021

Choose a reason for hiding this comment

josephsl commented Feb 6, 2021 via email

josephsl commented Feb 12, 2021

josephsl commented Feb 12, 2021

lukaszgo1 commented Feb 12, 2021

josephsl commented Feb 12, 2021 via email

lukaszgo1 Feb 13, 2021

Choose a reason for hiding this comment

josephsl commented Jan 31, 2021 •

edited

Loading