-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
[Windows] Can't import extension modules resolved via relative paths in sys.path #87271
Comments
If I attempt to import an extension module via something like this: from pkg import extmodule and it happens that "pkg" is found in a folder that is given in sys.path as a relative path, then the import fails, with ImportError: DLL load failed while importing extmodule: The parameter is incorrect. This only happens with 3.8 and later. AFAICS the reason is that if the module resolution is done via a relative path, it results in dynload_win.c:_PyImport_FindSharedFuncptrWindows attempting to feed LoadLibraryExW a relative path for the .pyd file. But LoadLibraryExW treats relative paths completely differently from absolute ones, in that it searches for the given path relative to the library search path entries rather than simply loading the DLL at that path. But as of 3.8 the current directory is removed from the search path, so the .pyd is never found. Since Python knows the specific file it wants LoadLibraryExW to load, having just resolved it via the import mechanism, I guess it ought to ensure it only ever calls LoadLibraryExW with an absolute path. (I'm assuming use of relative paths in sys.path is officially supported, since nothing I've found in the docs says otherwise.) |
I'd prefer that Python disallowed relative paths in sys.path [1]. But since they're allowed, I think importlib at least could try to resolve relative paths in a copy of sys.path before searching.
It happens to work prior to 3.8 even though the load uses the flag LOAD_WITH_ALTERED_SEARCH_PATH, for which it's documented that "[i]f this value is used and lpFileName specifies a relative path, the behavior is undefined". The implemented behavior with LOAD_WITH_ALTERED_SEARCH_PATH is that the directory of the given DLL filename is added to the head of the DLL search path, even though it's a relative path. Then the DLL filename is searched for like any other relative filename. For example, loading r"foo\spam.pyd" will try to load r"foo\foo\spam.pyd" (note the double "foo"), r"C:\Windows\System32\foo\spam.pyd", r"C:\Windows\System\foo\spam.pyd", r"C:\Windows\foo\spam.pyd", and so on. If the current working directory (i.e. ".") is in the DLL search path, and r"foo\spam.pyd" isn't accidentally found relative to a directory that's searched before ".", then the loader will find r".\foo\spam.pyd". Fortunately another thread can't change the working directory while the loader is searching, since the PEB lock is held. If r"foo\spam.pyd" is found and it depends on "eggs.dll", the loader will look for it first in the DLL directory, i.e. as r"foo\eggs.dll". The implicit inclusion of the working directory can be disabled or replaced with another directory via SetDllDirectoryW(), in which case the working directory will only be checked if %PATH% contains a "." entry. If it's replaced with another directory, then it's even inheritable from a SetDllDirectoryW() call in an ancestor process. 3.8+ uses the flag LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR, which requires the DLL filename to be a fully-qualified path. --- |
I agree with this fix. They can be resolved for each new import, if we think that's an important behaviour to preserve (might mess with the cache... but probably necessary if it's to be backported). But at some point sys.path entries need to be made absolute, and it definitely needs to happen before we try to load any DLLs. |
Added one possible change as a PR, but will need the importlib folk to weigh in on whether it's the best place. I also need to write a test (and possibly fix any tests that currently check that relative paths are _not_ resolved... letting CI handle that for me) |
Just needs the 3.8 backport - will get to that later tonight. |
The 3.8 backport is much more complicated, as we don't have access to the PathSkipRoot function there. So we can't use the native function. There's probably another way to implement the fix for 3.8, but I'm leaving that for another day. Feel free to chime in with suggestions and/or PRs. |
I guess you missed the comment that I left on the PR a few days ago. The 3.8 backport can use the older PathSkipRootW() in shlwapi.dll [1]. It works similarly, except it doesn't support a drive relative root such as "C:spam" -> ("C:", "spam"), but it's easy enough to handle that case in C. Also, it's documented that it's limited to MAX_PATH, but it works fine with long paths in Windows 10, even if the process does not have long-path support enabled. Anyway, just limit the copy to the first Examples: import ctypes
shlwapi = ctypes.WinDLL('shlwapi')
shlwapi.PathSkipRootW.restype = ctypes.c_wchar_p
path = (ctypes.c_wchar * 1000)() It returns NULL if there's no root: >>> path.value = r'spam'
>>> shlwapi.PathSkipRootW(path) is None
True Drive-relative paths aren't supported the same as they are in PathCchSkipRoot(): >>> path.value = r'C:spam'
>>> shlwapi.PathSkipRootW(path) is None
True Otherwise it seems to support the same range of paths as PathCchSkipRoot(): >>> path.value = r'\spam'
>>> shlwapi.PathSkipRootW(path)
'spam'
>>> path.value = r'C:\spam'
>>> shlwapi.PathSkipRootW(path)
'spam'
>>> path.value = r'\\server\share\spam'
>>> shlwapi.PathSkipRootW(path)
'spam'
>>> path.value = r'\\?\C:\spam'
>>> shlwapi.PathSkipRootW(path)
'spam'
>>> path.value = r'\\?\UNC\server\share\spam'
>>> shlwapi.PathSkipRootW(path)
'spam'
>>> path.value = r'\\?\Volume{12345678-1234-1234-1234-123456789ABC}\spam'
>>> shlwapi.PathSkipRootW(path)
'spam'
>>> path.value = r'\\.\PIPE\spam'
>>> shlwapi.PathSkipRootW(path)
'spam' [1] https://docs.microsoft.com/en-us/windows/win32/api/shlwapi/nf-shlwapi-pathskiprootw |
This caused a regression described in https://bugs.python.org/issue44061 |
There is a report about this change might have caused behaviour change for '.' in sys.path between 3.10.0a7 and 3.10.0b1 https://mail.python.org/archives/list/python-dev@python.org/thread/DE3MDGB2JGOJ3X4NWEGJS26BK6PJUPKW/ |
After this contribution, when using module at the root dir (maybe bad manners), the followings are expected behaviors? (1) relative drive in sys.path -> bytecode is not put in __pycache__ folder. >>> import sys
>>> sys.path.append('F:') # flash device, etc...
>>> import foo
>>> foo.__file__
'F:foo.py'
>>> foo.__cached__
'F:foo.cpython-311.pyc' (2) absolute drive in sys.path -> __pycache__ is under current dir, not absolute. >>> import sys
>>> sys.path.append('F:\\')
>>> import foo
>>> foo.__file__
'F:\\foo.py'
>>> foo.__cached__
'F:__pycache__\\foo.cpython-311.pyc' |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: