classification
Title: Enable better DLL resolution
Type: enhancement Stage: patch review
Components: Windows Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: brett.cannon, eric.snow, eryksun, jkloth, lukasz.langa, mattip, ncoghlan, paul.moore, ralf.gommers, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2019-02-23 00:28 by steve.dower, last changed 2019-03-19 07:31 by ralf.gommers.

Pull Requests
URL Status Linked Edit
PR 12302 open steve.dower, 2019-03-13 00:15
Messages (34)
msg336349 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-23 00:28
So the fundamental problem is that the default DLL search path on Windows changes in various contexts, and the only consistent approach is the most difficult to support with current packaging tools. The result is .pyd files that need to resolve .dll dependencies from directories *other* than where the .pyd file is located.

Here's a generic scenario:
* my_package.subpackage1.my_module is implemented as my_package/subpackage1/my_module.pyd
* my_package.subpackage2.my_module is implemented as my_package/subpackage2/my_module.pyd
* my_module.pyd in both cases depends on HelperLib.dll
* both modules must end up with the same instance of HelperLib.dll

While there are various ways for my_modules.pyd to locate HelperLib.dll, the only totally reliable way is to put HelperLib.dll alongside my_module.pyd. However, because it is needed twice, this means two copies of the DLL, which is unacceptable.

With Python 3.8, we are *nearly* dropping support for Windows 7, and I believe we can justify dropping support for Windows 7 without KB2533625 [1], which will have been released over eight years by the time 3.8 releases. This means the DLL search path enhancements are available.


Proposal #1: CPython calls SetDefaultDllDirectories() [2] on startup and exposes AddDllDirectory() [3] via the sys or os module.

This would ensure consistency in DLL search order regardless of security settings, and modules that have their own ".libs" directory have a supported API for adding it to the search path.

Past experience of forcing a consistent search path like this is that it has broken many users who expect features like %PATH% to locate DLL dependencies to work. For security reasons, this feature is already deprecated and often disabled (see [4]), so it can't be relied upon, but it makes it impossible for a single package to modify this setting or use the supported method for adding more DLL search directories.


Proposal #2: Resolve extension modules by full name

Without this proposal, the directory structure looks like:

my_package\
-subpackage1\
--__init__.py
--my_module.pyd
--HelperLib.dll
-subpackage2\
--__init__.py
--my_module.pyd
--HelperLib.dll

After this proposal, it could look like:

my_package\
-subpackage1
--__init__.py
-subpackage2\
--__init__.py
-my_package.subpackage1.my_module.pyd
-my_package.subpackage2.my_module.pyd
-HelperLib.dll

Essentially, when searching for modules, allow going up the package hierarchy and locating a fully-qualified name at any level of the import tree.

Note that since "import my_package.subpackage1.my_module" implies both "import my_package" and "import my_package.subpackage1", those have to succeed, but then the final part of the import would use subpackage1.__path__ to look for "my_module.pyd" and my_package.__path__ to look for "my_package.subpackage1.my_module.pyd".

This allows all extension modules to be co-located in the one (importable) directory, along with a single copy of any shared dependencies.

[1]: https://go.microsoft.com/fwlink/p/?linkid=217865
[2]: https://docs.microsoft.com/windows/desktop/api/libloaderapi/nf-libloaderapi-setdefaultdlldirectories
[3]: https://docs.microsoft.com/windows/desktop/api/libloaderapi/nf-libloaderapi-adddlldirectory
[4]: https://docs.microsoft.com/windows/desktop/Dlls/dynamic-link-library-search-order
msg336350 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-23 00:33
I nosied both Windows and import experts, and I'm about to go ping relevant numpy/scipy people on https://github.com/numpy/numpy/pull/13019

Personally, I would prefer option #1, despite the pain it would cause. It is the better long-term supported option, and Anaconda has already adopted a patch that does this. However, I think it's most appropriate to be a change in CPython at a major release boundary so that we can provide proper porting information for users.

Option #2 is kind of neat, and honestly I thought this already worked when the fully-qualified .pyd was in a folder on sys.path. However, it's going to mess big time with all of our existing build tools. So I'm not thrilled about passing on that kind of pain - then again, most people don't need this, and those who do can do their own hacks to make it work (on the theory that they're already applying their own hacks anyway).

I'm totally open to other suggestions on how to make these situations workable, though I will (continue to) push back hard against ideas that simply bring back the security concerns that led us to this point :)
msg336353 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-02-23 01:33
> Proposal #1: CPython calls SetDefaultDllDirectories() [2] on startup 

SetDefaultDllDirectories() affects the entire process, so it would needlessly break the world -- especially for embedding applications and ctypes users that have relied on adding directories to PATH. When loading an extension module, we can simply replace LOAD_WITH_ALTERED_SEARCH_PATH in the LoadLibraryExW flags with LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR and LOAD_LIBRARY_SEARCH_DEFAULT_DIRS (application directory, System32 directory, and directories added via SetDllDirectoryW and AddDllDirectory). Writers of extension modules are constrained by our API. We'll simply mandate that PATH is no longer searched.

It's cumbersome to require packages to have to manually call AddDllDirectory before being able to import an extension module with dependencies. We could create a protocol to store relative paths as an embedded resource in the extension module, which would be similar to the RPATH/RUNPATH $ORIGIN field in POSIX. We'd first map the extension module as a data file via LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE. Load and resolve the relative paths, add them via AddDllDirectory. Call LoadLibraryExW with the above-mentioned flags. Then remove the directories via RemoveDllDirectory.
msg336355 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-23 01:59
I'm very against doing magic to extract the names from the DLL, but maybe we could have a search path in the parent package? Then add/remove it around importing the module.

I think you're right that we just need to update the LoadLibrary flags, but will those also apply to dependencies of the loaded module? I thought not...
msg336371 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-02-23 06:49
> I'm very against doing magic to extract the names from the DLL, but 
> but maybe we could have a search path in the parent package? Then 
> add/remove it around importing the module.

That works, too. I'm happy either way.

We still can't load multiple DLLs with the same name with this technique. That requires private assembly packages, which is doable (in Windows 7+), but a bit complex and requires modifying the embedded #2 manifest of the extension module. The alternative is to rewrite the PE import tables of all DLLs to reference unique DLL names. Neither is necessary if everything is built against unique, versioned DLL names.

> I think you're right that we just need to update the LoadLibrary
> flags, but will those also apply to dependencies of the loaded 
> module? 

The DLL search path is computed once per LoadLibraryExW call based on either the call flags or the process default flags. We shouldn't mess with the process default, since there's no way to restore the legacy DLL search path, in particular this includes the Windows directory (%SystemRoot%), 16-bit System directory (%SystemRoot%\System), current directory, and PATH. 

Should we support a convenient syntax for including the current value of PATH at extension-module load time? This could be an entry that's exactly equal to "<PATH>". (Less-than and greater-than are reserved as wildcard characters by all Windows file systems that I can think of.) It would iterate over PATH, adding each directory via AddDllDirectory. Of course, all added directories would subsequently be removed via RemoveDllDirectory after the LoadLibraryExW call.
msg336380 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-23 14:06
> Should we support a convenient syntax for including the current value of PATH at extension-module load time? 

No. This is the bit that I refuse to add back, at least in CPython itself (if someone does it themselves then it's their bug). Private directories only.
msg336391 - (view) Author: mattip (mattip) * Date: 2019-02-23 16:57
Clear documentation would go a long way toward onboarding package providers. Of course this does not solve the problem for packages with no active ongoing support for windows, and will annoy developers whose code base is full of `os.environ['PATH']` games. Perhaps the solution should come with a deprecation warning when setting `os.environ['PATH']`.

It would be very helpful if failure to import a pyd (or for that matter to open a DLL with ctypes) due to missing support dlls could be easily debugged. Right now we get a message from windows that seems to suggest the file was not found.
- Python could check if the file exists on disk and print a more helpful message
- A debug hook to print the dll search path at the time of the failed LoadLibraryEx call, or perhaps adding it as an attribute of the Exception (this might be nice to have on Linux as well, even though there the Exception already includes the name of the missing *.so).

Even better would be official python/Microsoft support for a CLI version of depends.exe like ldd on linux, but that seems much harder and is out of scope for this issue.
msg336398 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-23 18:40
> Even better would be official python/Microsoft support for a CLI version of depends.exe like ldd on linux

The dumpbin.exe tool with /IMPORTS is a good start, and I've definitely wrapped it in Python before to do this kind of analysis (not reproducibly, yet...).

Doing this kind of analysis live is oddly tough, but there may be an ETW provider that a debug hook could enable to get more of a trace. Again, we're deep in third-party tool territory, not a change to core CPython.

Certainly if we can drop support for base Windows 7 we will document how to use more recent OS support via whatever we add. Though to a certain extent those hitting problems are going deep enough that reading the MSDN docs will have to be mandatory (at least for those who want to know "why"). I really don't want to have to reproduce those docs and make them guaranteed Python semantics.

> will annoy developers whose code base is full of `os.environ['PATH']` games. Perhaps the solution should come with a deprecation warning when setting `os.environ['PATH']`.

Yeah, this is the downside of changing anything at all, though of course since resolving DLLs via PATH is already broken, those developers are already annoyed ;) And we can't add warning that wouldn't annoy those who legitimately modify PATH for process launches. So I think it's mostly about providing a supported path for those developers to be able to port their code when they discover it's broken.
msg336408 - (view) Author: mattip (mattip) * Date: 2019-02-23 21:53
> legitimately modify PATH for process launches

Correct me if I'm wrong, don't process launches use the `env` kwarg for Popen, not the raw os.environ['PATH']?
msg336416 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-23 23:23
> Correct me if I'm wrong, don't process launches use the `env` kwarg for Popen, not the raw os.environ['PATH']?

If you don't provide env, it'll use the current process's environment, and if you do provide it without copying at least some entries, chances are your process won't start (and in general, you copy the current value and add to it). I've never seen anyone try to reset PATH here, nor would I recommend it.
msg336665 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-02-26 13:32
As a note in favour of the "Allow package nesting to be encoded in names, not just directories" approach, we actually have a similar problem affecting builtin modules: they're currently limited to top-level modules, with no way for the module to indicate that it's actually part of the parent package. Details at https://bugs.python.org/issue1644818 (yes, that's a SourceForge era issue number).

The solutions may not overlap in practice, but they're conceptually related (i.e. outside frozen modules, the package topology is pretty tightly coupled to the underlying filesystem layout)
msg337139 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-04 16:53
Adding Łukasz for his RM opinion on Win7 support for 3.8.

According to PEP 11, we've already dropped support for Win7 without Service Pack 1. Win7 SP1 would be dropped ~2-3 months after 3.8 releases, which means we still have to support it for all of 3.8.

My concern is the KB2533623 I mentioned in the original post, which adds the ability to control the search path properly. I *think* it might be already included in Win7 SP1, in which case we're fine (I'm confirming this with some colleagues), but if it's optional on top of SP1 then I want to make it required for Python.

Alternatively, I'm totally happy to make a three month exception to PEP 11 and just drop Win7 completely for 3.8. But I think that needs to be made official as early as possible, and should get python-dev exposure.

Łukasz - thoughts?

(Yes, I incorrectly typed the KB number at the top. Apparently I regularly fail to type numbers into bpo for some reason... doesn't happen elsewhere?)
msg337160 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-03-05 00:27
> Alternatively, I'm totally happy to make a three month exception to 
> PEP 11 and just drop Win7 completely for 3.8. But I think that needs 
> to be made official as early as possible

Windows 7 is still used on about 40% of Windows desktops, and will likely remain popular for a few years after its scheduled end of life in January 2020. Would this be a hard drop, i.e. would installing 3.8 be prevented in Windows 7? Or would it install but require users to manually install KB2533623?
msg337175 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2019-03-05 09:02
As someone whose work environment is still Windows 7, I'd prefer it if it were a soft desupport (i.e., we require users to manually ensure that the KB fix is installed, but we don't do anything specific to refuse to install Python on Win7).

I'd rather not drop Win7 support in 3.8 completely - I feel like that's a bit too aggressive, as Eryk says, there's still a *lot* of Windows 7 usage.
msg337223 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-05 16:33
> Would this be a hard drop, i.e. would installing 3.8 be prevented in Windows 7? Or would it install but require users to manually install KB2533623?

That's the question I'm asking :)

Python 3.9 is currently going to be a hard drop, according to our policy, and if Python 3.8 slips by 3 months then it will also be a hard drop unless we make an exception to the policy.

Paul's comment suggests we should avoid slipping/make the exception, and that it's okay to require the update, which is basically where I'm at too (provided the buildbot team are willing to keep an EOL'd OS running for as long as 3.8 is supported).
msg337705 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-11 22:29
In the absence of any other comments, here's my proposal.

* call SetDefaultDllDirectories() in Py_Main (i.e. not when embedded) to ensure secure search paths are always used
* ensure LoadLibrary when used in ctypes or importing will use the correct flags
* add sys._adddlldirectory() and sys._removedlldirectory() as CPython-specific functions for extending the search path (for use by packages currently modifying PATH at runtime)
* add check for KB2533623 to the installer and block if it is not present

Any thoughts? The only one I'm not 100% committed to is the SetDefaultDllDirectories call, but I'd rather ship it in alpha/beta releases and pull it out later if necessary. Passing the flags to LoadLibrary should have the same effect anyway, so I don't think changing the defaults in python.exe will make the current scenarios worse.
msg337722 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-03-12 05:48
> call SetDefaultDllDirectories() in Py_Main (i.e. not when embedded) 
> to ensure secure search paths are always used

That will require rewriting many scripts and packages that use ctypes or cffi to load DLLs. It would also break DLLs that internally rely on modifying PATH for a delayed load, though I hope that's uncommon. I think it's easier for everyone else if we implement this just for extension-module loading with the LoadLibraryExW flags. 

Also, if I'm understanding your intention, loading an extension may fail when Python is embedded if the process is using the legacy DLL search path. So, like with ctypes, we'll be forcing embedding applications to update how they load DLLs in order to comply with us, else they'll have to accept that some packages won't work without the SetDefaultDllDirectories call.

> ensure LoadLibrary when used in ctypes or importing will use the 
> correct flags

ctypes calls LoadLibraryW, which uses the default that's set by SetDefaultDllDirectories, if that's what we eventually decide is the best course of action.

If we decide to not call SetDefaultDllDirectories, then we should provide a way for ctypes packages to update to using the secure search path instead of relying on the legacy search path. We could rewrite the ctypes LoadLibrary wrapper to call LoadLibraryExW instead of LoadLibraryW and support the flags in the CDLL `mode` parameter, which is currently unused in Windows.

> add sys._adddlldirectory() and sys._removedlldirectory() as CPython-
> specific functions for extending the search path (for use by packages 
> currently modifying PATH at runtime)

I'd prefer some way for scripts and packages to configure their shared-library search paths as static data. The implementation would be kept private in the interpreter. 

I know there's debate about removing ".pth" files. But maybe we could  implement something similar for the DLL search path with package and script ".pthext" files. These would contain a list of directories (relative to the script or package) that extend the shared-library search path.

> add check for KB2533623 to the installer and block if it is not
> present

Also, at runtime we can raise a SystemError if AddDllDirectory isn't found via GetProcAddress. This supports portable Python installations.
msg337723 - (view) Author: mattip (mattip) * Date: 2019-03-12 07:12
Correct me if I'm wrong, but once SetDefaultDllDirectories() is used, there is no going back: PATH no longer can change the search path no matter what flags are used with LoadLibrary* calls (see the recent Anaconda issue when they did this and broke NumPy). Assuming that is true, then

> add sys._adddlldirectory() and sys._removedlldirectory() as CPython-specific functions for extending the search path (for use by packages currently modifying PATH at runtime)

Why is this CPython-specific and "private"? It seems like
* it should be a public interface, used by all implementations consistently, as a policy decision for the win32 platform and documented as such
* located in os, much like os.environ (not critical)

There should be some kind of debugging tool for when LoadLibraryEx fails, to indicate what might have gone wrong, perhaps os.getdlldirectory() would be helpful
msg337728 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2019-03-12 09:14
> Also, if I'm understanding your intention, loading an extension may fail when Python is embedded if the process is using the legacy DLL search path. So, like with ctypes, we'll be forcing embedding applications to update how they load DLLs in order to comply with us, else they'll have to accept that some packages won't work without the SetDefaultDllDirectories call.

This bothers me - how will backward compatibility work in that case?
There are applications (for example, Vim) that can embed Python, and
it's possible to pick the Python version at compile time. Would Vim
need additional code (possibly guarded by some sort of "If this is
Python 3.8 or later" flag, which from my knowledge of the Vim code
would not be particularly easy to add in a backward compatible way) to
handle this change?

Actually, as a more general point, I have been following this
discussion, but I really have no feel as to what practical impact
there would be on an embedded application. I get that this is OS
functionality, and therefore it's not Python's place to explain the
details to users, but IMO it *is* Python's responsibility to explain
how embedding applications will need to change if we change how we
configure things. Even if users are currently using an approach that
is no longer encouraged (which is *I think* what you're saying about
putting DLLs on PATH) they are still using something that works right
now, and we're breaking it - so we need to describe what changed,
*why* we felt we should break their code, and what they need to do,
both to switch to the new model, and (if they have a requirement to do
so) to support the old and new models simultaneously in their code
(very few people, not even embedders, can suddenly say "we're dropping
support for Python 3.7 and older, we only allow 3.8+ from now on").
msg337749 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-12 15:15
> That will require rewriting many scripts and packages that use ctypes or cffi
> to load DLLs. It would also break DLLs that internally rely on modifying PATH
> for a delayed load, though I hope that's uncommon. I think it's easier for
> everyone else if we implement this just for extension-module loading with the
> LoadLibraryExW flags.

Only if they're loading them via PATH. If they're using full paths they'll be fine, and if they're using system DLLs they'll be fine. In both cases, the fix will work (better) with existing versions.

> Also, if I'm understanding your intention, loading an extension may fail when
> Python is embedded if the process is using the legacy DLL search path.

That's true. "import" will always use the secure flags, and so if you were relying on PATH to locate dependencies of the extension module (note that extension modules themselves are loaded by full path, so it doesn't apply to them), you need to stop doing that.

> Also, at runtime we can raise a SystemError if AddDllDirectory isn't found via
> GetProcAddress. This supports portable Python installations.

This adds a lot of complexity for very old Windows 7 installs. I'm not inclined to care that much for them - installing security updates isn't a big ask for a nearly EOL operating system.

> Correct me if I'm wrong, but once SetDefaultDllDirectories() is used, there is
> no going back: PATH no longer can change the search path no matter what flags
> are used with LoadLibrary* calls

Correct. But this is already the case if your sysadmin has enabled certain policies or if you're running via Store Python. So you can't rely on PATH anyway.

> Why is this CPython-specific and "private"? It seems like
> * it should be a public interface, used by all implementations consistently,
> as a policy decision for the win32 platform and documented as such

Not every implementation has to support Windows. We can certainly recommend that those that do copy it, but I'm not going to force MicroPython to declare an exception from a standard Python API.

> * located in os, much like os.environ (not critical)

Fair point. It can go into os. :)

> There should be some kind of debugging tool for when LoadLibraryEx fails, to
> indicate what might have gone wrong, perhaps os.getdlldirectory() would be
> helpful

I'd love to have this. Now someone just has to invent one that can be used from Python :) It's unrelated to this discussion - in fact, this change will make it so that you get the failure on _all_ machines, not just on some random user's machine.

We can't retrieve the true search path, only the ones that were added through an API that we control and making assumptions based on the documentation. I think this would be more of a distraction. The best way to diagnose actual LoadLibrary failures is to use a debugger, at which point simply getting the search paths doesn't add anything.

> This bothers me - how will backward compatibility work in that case?

The new search order is compatible with the old search order, so you can update all your layouts to have DLL dependencies in suitable locations and you'll be fine.

And if you're still writing code for Windows 7 with no security updates installed, Python 3.8 isn't going to save you anyway.

> I really have no feel as to what practical impact there would be on an
> embedded application.

Since we're not going to change the default search directories for the entire process when embedding, the only practical impact is that your extension modules need to have their dependent DLLs either:
* in the system directory
* adjacent to the .pyd file
* in a directory added using AddDllDirectory

And if the embedding application is already calling SetDefaultDllDirectories, as has been recommended for years, then they're already experiencing this change and won't have to update a thing.
msg337760 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-12 15:45
Since I just dug enough to find it, the best way to diagnose problems with dependent DLLs not being found is probably to run Process Monitor [1] while doing the import and checking its logs. It should show the paths that were attempted to be accessed.

[1]: http://technet.microsoft.com/en-us/sysinternals/bb896645
msg337765 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2019-03-12 15:55
> > This bothers me - how will backward compatibility work in that case?
>
> The new search order is compatible with the old search order, so you can update all your layouts to have DLL dependencies in suitable locations and you'll be fine.

OK, cool. But one thing I'm not clear on, will this change just affect
the embedded Python, or will it affect the whole process - which would
mean that supporting an embedded Python 3.8 interpreter would mean
potentially reorganising the application layout. That may be quite a
cost, in some applications.

Note that this is all on the basis of "I don't understand the
implications, they should be documented" rather than being a specific
problem that I know will happen. My particular scenario, though, is an
application like Vim, that provides optional support for an "embedded
scripting" which may be any one of a number of Python versions, or
even other languages. In an application like that, costs for
supporting Python 3.8 may simply result in no (or delayed) support for
Python 3.8, rather than the application getting fixed.

> And if you're still writing code for Windows 7 with no security updates installed, Python 3.8 isn't going to save you anyway.

Nobody's suggesting that it will. But maintaining *existing* code that
supports older Windows versions, while still allowing Python 3.8 to be
used as an embedded scripting language on systems that support it, is
an entirely reasonable proposal.

> > I really have no feel as to what practical impact there would be on an
> > embedded application.
>
> Since we're not going to change the default search directories for the entire process when embedding

OK, if that's the case, then that alleviates most of my concerns. But
it really wasn't obvious to me, and it's something that I think should
be made clear in the docs, if only to reassure embedding applications
that Python isn't making global changes. The docs for SetDllDirectory
seem to imply that there *is* a global impact - "The SetDllDirectory
function affects all subsequent calls to the LoadLibrary and
LoadLibraryEx functions" (note - *all* subsequent calls, which implies
that behaviour will change for the embedding application once Python
has been loaded).

> the only practical impact is that your extension modules need to have their dependent DLLs either:
> * in the system directory
> * adjacent to the .pyd file
> * in a directory added using AddDllDirectory

That seems fine, so let's just state that and keep things simple for
embedders to understand.

> And if the embedding application is already calling SetDefaultDllDirectories, as has been recommended for years, then they're already experiencing this change and won't have to update a thing.

Sadly, in my experience, an awful lot of projects (specifically, open
source projects that write mostly cross-platform code, with the
minimum of OS-specific differences) don't follow recommendations like
this. They use LoadLibrary without digging too deeply into the
implications or complexities, as long as it does what they want. And I
don't think MS helped themselves much here, either - the whole
business with SxS installs and assemblies was (IMO) *way* too much
complexity for most cross platform projects to bother with, and went
ignored. Even once things got simpler again, there remained a sense of
"don't go there, just get something that works". (And to be clear, I'm
not bashing on MS here - I find the Linux machinery around all of this
to be just as complex and confusing).

Anyhow, if as you say the only impact is that when a pyd file depends
on a DLL, that DLL needs to be located in one of three places, all of
which are equally valid on Python <=3.7, and there's no impact on the
non-Python part of the embedded application, then it's not a big deal.
Let's make the change, write up those points in What's New (at least),
and leave it at that.
msg337790 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-03-12 18:53
> will this change just affect the embedded Python, or will it affect 
> the whole process

SetDefaultDllDirectories affects the whole process and cannot be reverted back to the legacy search path that includes "%SystemRoot%",  "%SystemRoot%\System" (the old 16-bit directory), the current working directory, and the PATH directories. Also, there's no LoadLibraryExW flag to use the legacy search path, either, so scripts and packages that use ctypes or cffi will have to be updated if they depend on PATH or changing the working directory. 

The alternative is to modify Python's importer to use the secure LoadLibraryExW flags (i.e. LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS), without affecting the rest of the process. 

LOAD_LIBRARY_SEARCH_DEFAULT_DIRS includes the application directory, the user directories added with AddDlldirectory or SetDllDirectoryW, and the System32 directory. LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR prepends the directory of the target DLL, which must be passed as a fully-qualified path.

> The docs for SetDllDirectory seem to imply that there *is* a global
> impact

SetDllDirectoryW still works after calling SetDefaultDllDirectories, as long as we include either LOAD_LIBRARY_SEARCH_USER_DIRS or LOAD_LIBRARY_SEARCH_DEFAULT_DIRS. It only allows adding a single directory, so it's of limited use anyway.
msg337793 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-12 19:45
> The alternative ...

Is what I proposed in the first place. Adding the SetDefaultDllDirectories call to Py_Main (that is, NOT for embedders) is to ensure consistency throughout the process. It'll only affect extension module dependencies that do their own delay loading and currently rely on unsupported resolve paths.

Since everyone seems to have misunderstood at least part of the proposal, I'm not going to explain any more until I have a patch. Excluding boilerplate and docs, it'll be about ten lines of code.
msg337795 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2019-03-12 20:09
OK, I don't really follow enough of the details here to comment properly. But clearly Steve and Eryk are not yet in agreement.

My personal view is that this is something where we should be trying *very* hard to preserve backward compatibility. The proposal here is intended to solve the problem of making it easier for .pyd files to reliably load helper DLLs from shared locations. That's fine, and while it's an important use case (AIUI, it matters for a lot of the scientific stack) IMO it's *not* important enough to warrant breaking working scripts or embedding applications (particularly as this is a fairly obscure detail of how Windows works, so it's unlikely that people carefully follow "best practices" here).

I'm very concerned that comments I've seen here, specifically

>> That will require rewriting many scripts and packages that use ctypes or cffi
>> to load DLLs. It would also break DLLs that internally rely on modifying PATH
>> for a delayed load, though I hope that's uncommon. I think it's easier for
>> everyone else if we implement this just for extension-module loading with the
>> LoadLibraryExW flags.
>
> Only if they're loading them via PATH. If they're using full paths they'll be fine, and if they're using system DLLs they'll be fine. In both cases, the fix will work (better) with existing versions.
>
>> Also, if I'm understanding your intention, loading an extension may fail when
>> Python is embedded if the process is using the legacy DLL search path.
>
> That's true. "import" will always use the secure flags, and so if you were relying on PATH to locate dependencies of the extension module (note that extension modules themselves are loaded by full path, so it doesn't apply to them), you need to stop doing that.

imply that it's OK to break working code "because they are doing things wrongly". That's not how backward compatibility works - we should avoid breaking *any* working code, no matter how ill-advised it seems to be.

If it's necessary to break code that (say) uses ctypes to load a DLL via PATH, or an embedding application that relies on getting DLLs using PATH, then we need to follow PEP 387 and go through a deprecation cycle for the existing behaviour.

For the ctypes case I assume we can detect where we found the DLL being loaded, so warning that behaviour will change is certainly possible.

For the embedding case, we could (for example) add an API Py_UseSecureSearchPath(bool) that embedders should call to opt into the new search semantics. With an explicit opt-in, we can then migrate that to be the default over time - have the Python API warn for a release if called without the opt-in, and then switch the default to be the secure search path, with applications that want to use the old search path being able to opt out using Py_UseSecureSearchPath(FALSE) for a release or two.

That proposal is very much off the top of my head. But the point is that it's not impossible to make the transition follow the normal backward compatibility rules, and so we should do so.

Of course, far simpler would be to choose a solution which *doesn't* break existing code :-)
msg337796 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2019-03-12 20:10
> Since everyone seems to have misunderstood at least part of the proposal, I'm not going to explain any more until I have a patch. Excluding boilerplate and docs, it'll be about ten lines of code.

+1 on that. Code is much harder to misunderstand :-)

Paul
msg337801 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-03-12 20:26
Okay. Sorry for adding noise. My mental hiccup was in thinking it would continue to use LOAD_WITH_ALTERED_SEARCH_PATH in conjunction with SetDefaultDllDirectories: LOAD_LIBRARY_SEARCH_DEFAULT_DIRS. I forgot that it's documented that they shouldn't be combined. Instead we have to explicitly use LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS in each LoadLibraryExW call in order to support loading DLLs beside the extension module. In this case, embedding applications that don't call SetDefaultDllDirectories won't have a problem loading extensions that rely on AddDllDirectory. It's only ctypes and cffi packages that will be forced to update if they currently rely on PATH or the working directory.
msg337805 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-12 20:51
Actually, CFFI is probably going to be most affected. Who do we know from that project who should be nosied on this?
msg337807 - (view) Author: mattip (mattip) * Date: 2019-03-12 20:59
I have left a note for arigato, but why is CFFI different than ctypes or c-extension modules? All will need adjustment.
msg337809 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-12 21:12
It's different from ctypes because I can update ctypes as part of this change :)

The reason it matters is that it's basically a wrapper around LoadLibrary, and that's what is going to change here. Hopefully we won't cause too much trouble for their users.
msg337826 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-03-13 00:21
PR has been posted, but it's incomplete (docs, news, etc.)

And unfortunately longer than I'd hoped, since we have to use GetProcAddress for these function on Windows 7 still (even if it has the required update), but since it's coming from kernel32 (which is always loaded) and these are going to be rare calls I'm not too concerned. Still, as soon as we drop Win7, it'll be nice to clean these up.

I ended up making the functions public as os.add_dll_directory and os.remove_dll_directory. The return value is using a capsule (which is needed because it's an opaque pointer that you use to remove the directory later), and honestly I don't think it matters enough to give it its own type. Given the choice between making the functions private (and requiring "import nt; nt._add_dll_directory()") vs. implementing a whole type for one opaque value, I'll make the functions private :)
msg337838 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-03-13 06:50
> Since I just dug enough to find it, the best way to diagnose problems 
> with dependent DLLs not being found is probably to run Process Monitor 
> [1] while doing the import and checking its logs. It should show the 
> paths that were attempted to be accessed.

Don't forget loader snaps, which we can log using a standard debugger such as WinDbg or by attaching a Python script as a debugger (e.g. debug a child process via the DEBUG_PROCESS creation flag). For the latter, we need a debug-event loop (i.e. WaitForDebugEventEx via ctypes) that logs debug-string events. This will show the paths that the loader checks and the load attempts that fail with STATUS_DLL_NOT_FOUND (0xC0000135). We have to first enable loader snaps for the executable by setting a flag value of 2 in the "GlobalFlag" DWORD in the key "HKLM\Software\Microsoft\Windows NT\CurrentVersion\Image File Execution Options\<executable name>". Or use gflags.exe to set this value.
msg337856 - (view) Author: mattip (mattip) * Date: 2019-03-13 16:03
@eryksun - is there a sample resource: blog post, code snippet, msdn documentation, that demonstrates how that all works? 

I personally find the MSDN documentation of "what happens when I call LoadLibraryEx" not very user friendly. They seem to be written to document the system calls and not to explain the user experience. A diagram with some examples of setting and debugging this would go a long way to helping users enter the right mindset to debug failures to load DLLs because the support dlls they depend on are not found
msg338213 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-03-18 12:38
Personally I am fine with Python 3.8 dropping Windows 7 support entirely if this makes it work better in Windows 8+. However, the 3 month overlap here would set a precedent that we don't have to adhere to self-imposed timing restrictions which is dangerous territory.

I think it's reasonable to leave Windows 7 support but *require* KB2533625 to be in. We've done similar things before on other platforms.
History
Date User Action Args
2019-03-19 07:31:43ralf.gommerssetnosy: + ralf.gommers
2019-03-18 12:38:07lukasz.langasetmessages: + msg338213
2019-03-13 16:03:37mattipsetmessages: + msg337856
2019-03-13 06:50:36eryksunsetmessages: + msg337838
2019-03-13 00:21:10steve.dowersetmessages: + msg337826
2019-03-13 00:15:15steve.dowersetkeywords: + patch
stage: patch review
pull_requests: + pull_request12277
2019-03-12 21:12:12steve.dowersetmessages: + msg337809
2019-03-12 20:59:40mattipsetmessages: + msg337807
2019-03-12 20:51:00steve.dowersetmessages: + msg337805
2019-03-12 20:26:35eryksunsetmessages: + msg337801
2019-03-12 20:10:46paul.mooresetmessages: + msg337796
2019-03-12 20:09:12paul.mooresetmessages: + msg337795
2019-03-12 19:45:47steve.dowersetmessages: + msg337793
2019-03-12 18:53:57eryksunsetmessages: + msg337790
2019-03-12 15:55:45paul.mooresetmessages: + msg337765
2019-03-12 15:45:21steve.dowersetmessages: + msg337760
2019-03-12 15:15:03steve.dowersetmessages: + msg337749
2019-03-12 09:14:22paul.mooresetmessages: + msg337728
2019-03-12 07:12:24mattipsetmessages: + msg337723
2019-03-12 05:48:03eryksunsetmessages: + msg337722
2019-03-11 22:29:49steve.dowersetmessages: + msg337705
2019-03-05 16:33:16steve.dowersetmessages: + msg337223
2019-03-05 09:02:48paul.mooresetmessages: + msg337175
2019-03-05 00:27:35eryksunsetmessages: + msg337160
2019-03-04 16:53:56steve.dowersetnosy: + lukasz.langa
messages: + msg337139
2019-02-26 13:32:49ncoghlansetmessages: + msg336665
2019-02-23 23:23:44steve.dowersetmessages: + msg336416
2019-02-23 21:53:15mattipsetmessages: + msg336408
2019-02-23 18:40:56steve.dowersetmessages: + msg336398
2019-02-23 16:57:47mattipsetnosy: + mattip
messages: + msg336391
2019-02-23 14:06:00steve.dowersetmessages: + msg336380
2019-02-23 06:49:42eryksunsetmessages: + msg336371
2019-02-23 01:59:10steve.dowersetmessages: + msg336355
2019-02-23 01:33:34eryksunsetnosy: + eryksun
messages: + msg336353
2019-02-23 01:00:01jklothsetnosy: + jkloth
2019-02-23 00:33:04steve.dowersetmessages: + msg336350
2019-02-23 00:28:57steve.dowercreate