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

dll issue for python 3.8+ #235

Closed
thomasmfish opened this issue Feb 17, 2022 · 13 comments
Closed

dll issue for python 3.8+ #235

thomasmfish opened this issue Feb 17, 2022 · 13 comments

Comments

@thomasmfish
Copy link
Contributor

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 (maybe os.path.dirname(microscope.__file__) instead of os.getcwd()?)

@thomasmfish
Copy link
Contributor Author

Linking this very detailed StackOverflow answer for later

@carandraug
Copy link
Collaborator

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:

  1. does ctypes.util.find_library finds the DLLs you are looking for? (the directory with the DLLs should be in the PATH).
  2. if you use ctypes.WinDLL(..., windmode=0) does it work?

@thomasmfish
Copy link
Contributor Author

thomasmfish commented Mar 14, 2022

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.

@thomasmfish
Copy link
Contributor Author

thomasmfish commented Mar 14, 2022

(I'm going to test winmode=0 too.)

@thomasmfish
Copy link
Contributor Author

winmode=0 wins the day! I'll quickly make the changes now

@thomasmfish
Copy link
Contributor Author

#237 Should be an adequate fix. Feel free to suggest/make any changes if you'd prefer it done differently.

@iandobbie
Copy link
Collaborator

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.

@thomasmfish
Copy link
Contributor Author

**{} 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?

@iandobbie
Copy link
Collaborator

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.

@thomasmfish
Copy link
Contributor Author

thomasmfish commented Mar 29, 2022 via email

@thomasmfish
Copy link
Contributor Author

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)

@iandobbie
Copy link
Collaborator

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.

carandraug pushed a commit that referenced this issue Aug 22, 2023
Since Python 3.8, ctypes uses an altered search path when loading DLLs
in Windows which only includes the application directory, the system32
directory, and directories manually specified with add_dll_directory.
None of those suit us.  The standard search path can be specified with
`winmode=0`.

This commit adds a new function to loads libraries with `winmode=0`
and makes all modules that load shared libraries use it.
@carandraug
Copy link
Collaborator

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 03e6b5c . 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:

  1. application directory
  2. system32 directory
  3. directories we manually specify with 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).

thomasmfish added a commit to thomasmfish/microscope that referenced this issue Aug 30, 2023
commit c2b3dc5
Author: Ian Dobbie <[email protected]>
Date:   Tue Aug 22 12:21:54 2023 -0400

    Added the required getValues function to the raspberrypi valueLogger module.

commit fa592d6
Author: David Miguel Susano Pinto <[email protected]>
Date:   Tue Aug 22 13:52:42 2023 +0100

    RPiValueLogger: check for availability of MCP9808 and TSYS01 classes.

commit bcd3d86
Author: David Miguel Susano Pinto <[email protected]>
Date:   Tue Aug 22 13:44:11 2023 +0100

    PiCamera: use ROI field names instead of tuple index for readability

commit 8f631d7
Author: David Miguel Susano Pinto <[email protected]>
Date:   Tue Aug 22 13:42:07 2023 +0100

    maint: black and isort for consistent style.

commit 03e6b5c
Author: David Miguel Susano Pinto <[email protected]>
Date:   Tue Aug 22 12:42:09 2023 +0100

    Use a standard search path for DLL loading in windows (python-microscope#235)

    Since Python 3.8, ctypes uses an altered search path when loading DLLs
    in Windows which only includes the application directory, the system32
    directory, and directories manually specified with add_dll_directory.
    None of those suit us.  The standard search path can be specified with
    `winmode=0`.

    This commit adds a new function to loads libraries with `winmode=0`
    and makes all modules that load shared libraries use it.

commit 6a4ee44
Author: Ian Dobbie <[email protected]>
Date:   Mon Aug 21 14:01:48 2023 -0400

    Fix picamera roi code to use the ROI named tuples and then just return roi python-microscope#279

commit 76a6364
Author: Ian Dobbie <[email protected]>
Date:   Mon Jul 24 22:09:11 2023 -0400

    Added pulldata option to allow cockpit to pull data from valuelogger devices
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants