Closed thomasmfish closed 1 year ago
Linking this very detailed StackOverflow answer for later
I don't think that answer in StackOverflow is correct anymore. It might have been correct at the time it was written (2014) but the winmode
parameter was added to ctypes.CDLL
in Python 3.8 (but note that the documentation for what is the default winmode
value in the Python documentation is wrong, see python/cpython#19167).
Tom, can you check:
ctypes.util.find_library
finds the DLLs you are looking for? (the directory with the DLLs should be in the PATH
).ctypes.WinDLL(..., windmode=0)
does it work?From the microsoft docs website, the standard DLL search order for desktop applications is:
- The directory from which the application loaded.
- The current directory.
- The system directory. Use the GetSystemDirectory function to get the path of this directory.
- The 16-bit system directory. There is no function that obtains the path of this directory, but it is searched.
- The Windows directory. Use the GetWindowsDirectory function to get the path of this directory.
- The directories that are listed in the PATH environment variable. Note that this does not include the per-application path specified by the App Paths registry key. The App Paths key is not used when computing the DLL search path.
The first two aren't included by Python 3.8+, which only searches PATH
. This is also the case when using find_library
, which seems to use the same logic as WinDLL but returning a path rather than loading the library.
This leads to the question of where, ideally, should the DLL(s) be?
The microscope package path isn't automatically in the PATH
. I haven't tried this but from the documentation, it looks like an alternative method would be to add MICROSCOPE_PATH
to the PATH
, then set/update MICROSCOPE_PATH
when loading microscope. That seems clunkier than using a context manager to add to a dll directory, since MICROSCOPE_PATH
would persist after closing.
(I'm going to test winmode=0
too.)
winmode=0
wins the day! I'll quickly make the changes now
A quick look and I only have one thought. Is there ever a possibility that kwargs might be populated before you do this? If so then we need to test for this before setting it to an empty dict.
**{}
does nothing - you can add as many , **{}
s you'd like into a function call without anything happening, e.g.:
def fn(*args, **kwargs):
print("args: %s; kwargs: %s" %(args, kwargs))
fn(**{}, **{}, **{})
>>> args: (); kwargs: {}
fn(**{}, **{}, **{}, **{"lemon": 123})
>>> args: (); kwargs: {'lemon': 123}
Is that what you mean, or am I misunderstanding?
No you are misunderstanding. I am saying what happens if somewhere else in the code someone is passing in parameters via kwargs. This code overwrites the kwargs with no test to see if it already exists. This might be difficult to diagnose, might be sensible to use a different variable name.
Ah, I see what you mean. I did check upstream of that but I am thinking that maybe a DLL loading function should be put somewhere sensible in microscope to avoid the duplication of code, which would also keep the kwarg
variable local. I'll fiddle about later or tomorrow
From: Ian Dobbie @.> Sent: Tuesday, March 29, 2022 6:24:20 PM To: python-microscope/microscope @.> Cc: Fish, Thomas (DLSLtd,RAL,LSCI) @.>; Author @.> Subject: Re: [python-microscope/microscope] dll issue for python 3.8+ (Issue #235)
No you are misunderstanding. I am saying what happens if somewhere else in the code someone is passing in parameters via kwargs. This code overwrites the kwargs with no test to see if it already exists. This might be difficult to diagnose, might be sensible to use a different variable name.
— Reply to this email directly, view it on GitHubhttps://github.com/python-microscope/microscope/issues/235#issuecomment-1082164334, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL6IKWYCGF5623P2Q527U4LVCM4EJANCNFSM5OVV2QPQ. You are receiving this because you authored the thread.Message ID: @.***>
-- This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom
Okay, I've added a new commit that refactors library loading into a general function in _utils.py. Let me know what you think (latest commit is here)
Thats a good idea, just a quick dllLoad function that can do the correct magic and make it easier to dix if the python, windows etc, changes break the path finding again.
I've looked at @thomasmfish fix but it assumes that under Windows one always uses WinDLL
or CDLL
based on whether we're on Windows or not which is not true (see #261). I made a similar approach (new util function and replaced all use of WinDLL
and CDLL
with it) in 03e6b5c94e3f . In addition to Tom's approach, this provides a path for callers to specify extra kwargs to CDLL
and prevents winmode
from being overwritten (if caller really wants to).
This restores the use of windows default search path and fixes this issue. Closing as fixed.
Some thoughts on taking this approach. Python 3.8 uses an altered search path. There appears to be two reasons for it, 1) security (I guess that's because PATH
can't be trusted) and 2) consistency (I'm not sure how but it seems that it made os.add_dll_directory
not work sometimes --- see documentation for it). Bottom line is, since Python 3.8 in Windows, the search path is:
os.add_dll_directory
Which is no good for us. The dll we want to use are in none of those places and we can't expect users to be able to move them there. Why? Because we can't guess where the dlls are, and we can't rely on the users knowing where the dlls and its dependencies are (which means we can't expect them to move them to a specific place either). We can only rely on the installer of the libraries we wrap to install the dlls somewhere and add them to PATH
(or document how other programs can make use of them).
The discussion in Python is in BPO-36085 and is a bit long. There's plenty of discussion but it seems the "only?" use case they're considering is Python packages with dlls on it and not Python packages that wrap libraries that they expect the system to have (which is what we are).
https://bugs.python.org/issue43173
This can be fixed with a simple:
with os.add_dll_directory(os.getcwd()):
before any code that imports dlls, or something similar (maybeos.path.dirname(microscope.__file__)
instead ofos.getcwd()
?)