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

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 :)

> 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.

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...

> 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.

> 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.

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.

> 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.

> 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']?

> 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.

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)

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?)

> 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?

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.

> 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).

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.

> 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.

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

> 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").

> 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.

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

> > 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.

> 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.

> 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.

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 :-)

> 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

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.

Actually, CFFI is probably going to be most affected. Who do we know from that project who should be nosied on this?

I have left a note for arigato, but why is CFFI different than ctypes or c-extension modules? All will need adjustment.

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.

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 :)

> 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.

@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

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.

I think we'll be keeping Win7 with the KB.

However, we've discovered in the PR that changing the default DLL lookup may cause Python to crash when accessing HKEY_PERFORMANCE_DATA (which fails to delay-load a DLL). This occurs because accessing that key enumerates a set of installed services (presumably both 1st and 3rd party) and one of those fails on AppVeyor. (The Azure Pipelines tests are fine, as are all the local test machines I've used.) There's no indication what AppVeyor has installed that is causing the problem.

So it seems we'll have to not use the safe DLL lookup for all parts of CPython, and restrict it only to ctypes and extension module loading. (Or else drop AppVeyor as a required check.)

I've found AppVeyor's support forum (https://help.appveyor.com/) to be fairly responsive; it may be worth asking them about the issue there.

I added some logging for the AppVeyor build at https://ci.appveyor.com/project/python/cpython/builds/23258953

Looks like the offending DLLs are:
- perf-MSSQL$SQL2017-sqlctr14.0.1000.169.dll
- perf-MSSQL$SQL2016-sqlctr13.1.4474.0.dll

Since the events are pulled out in reverse order, I assume the first one is the problem. I'll see if I can ping the MSSQL team and find out if they know what's going on.

Since there's no chance of getting old SQL Server fixed, I think we should just merge it without the SetDefaultDllDirectories change.

Any comments, questions or more feedback on the PR?

> Any comments, questions or more feedback on the PR?

I commented on the PR that I'm concerned that ctypes.CDLL will no longer open a path with slashes in it (e.g. CDLL("./spam.dll") or CDLL("eggs/spam.dll")) relative to the working directory, and may accidentally match another directory in the search path. In POSIX, a path with a slash in it is always relative to the working directory and never searched for. In Windows, particularly for the loader, all relative paths are always searched for. This works with the current LoadLibraryW call, with minimal risk of name collisions, because only the application directory and system directories are checked before the working directory, which is checked before searching PATH. I suggest that with the change to use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS, ctypes should first resolve such a path by calling GetFullPathNameW, as is suggested in the Windows docs.

I also had suggested documenting and exposing a subset of the loader flags for use with the CDLL `mode` parameter, which is currently unused in Windows. This would make it convenient for a user to include the flag LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR when the DLL path is fully qualified. However, that was deemed too peripheral to be worth the extra effort of supporting and documenting the parameter and adding the required tests, which is understandable. This can maybe be addressed in another issue, if the need arises. That said, it would be nice to provide parity with C extension modules here, which are always loaded with the latter flag. If given a fully-qualified path, or a relative path with slashes in it that's resolved via GetFullPathNameW, _ctypes.LoadLibrary could automatically include the LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR flag.

I implemented the feature Eryk was asking for, as it's also by far the easiest way to test the add_dll_directory function. So now ctypes.CDLL (and subclasses) have a `winmode` argument that gets turned directly into LoadLibraryEx flags (changing `mode` would have required a deprecation period, as it's explicitly documented as being ignored on Windows).

I believe the docs are updated as much as necessary, but if I missed something please call out.

There are no specific "import" tests, because it's such a pain to set those up (I need to delete files from the build directory during the test, and if other tests have already used them that will fail, or I need to copy the Python install elsewhere so it doesn't pick those up). But with ctypes I can exclude the application directory from the search paths. Open to brilliant ideas here.

> There are no specific "import" tests, because it's such a pain to set 
> those up (I need to delete files from the build directory during the 
> test, and if other tests have already used them that will fail, or I 
> need to copy the Python install elsewhere so it doesn't pick those
> up). 

Instead of copying the whole install, you should be able to symlink the core binaries (e.g. python.exe, python38.dll, python3.dll, vcruntime140.dll) to a temporary directory and set PYTHONHOME. Most (or at least some) of the build bots should be set up to grant the symlink privilege to the current user or all standard users, and %TEMP% should be on an NTFS/ReFS volume that supports reparse points.

So symlinking didn't work (Python is too clever for that these days ;) ), but straight copying the exe and required DLLs is fine.

It puts python.exe, python38.dll and vcruntime140.dll (properly discovered of course) into a temp directory, puts _sqlite3.pyd into a subdirectory and only allows imports from that directory and the pure stdlib (for encodings). Then we test with add_dll_directory(), then copy sqlite3.dll in and test again without.

Assuming tests all pass, I consider this complete now.

New changeset 2438cdf0e932a341c7613bf4323d06b91ae9f1f1 by Steve Dower in branch 'master':
bpo-36085: Enable better DLL resolution on Windows (GH-12302)
https://github.com/python/cpython/commit/2438cdf0e932a341c7613bf4323d06b91ae9f1f1

Leaving this in commit review for a couple of days, then I'll close.

Acknowledging the buildbot failure at https://buildbot.python.org/all/#builders/12/builds/2181

I'll try and take a look today. Apparently Windows 8 has a slightly different understanding of the flags used in ctypes tests?

Pushed a potential fix, assuming there's something weird going on with relying on cwd rather than full paths (I know there's some weirdness here when paths get too long, for example).

Zach - this is one of your buildbots. Can we trigger a run from my branch? (As an aside, https://buildbot.python.org/all/#/builders?tags=custom.unstable&tags=custom.stable is currently showing no builders, but I'm not sure we have a custom one that matches the configuration here anyway.)

Try https://buildbot.python.org/all/#/builders?tags=%2Bcustom instead :)

You can trigger a build by pushing to the `buildbot-custom` branch and if need be I can grant you SSH or RDP access to that worker, just let me know.

Guess the link in the devguide needs fixing... I'm out for the next few hours but will give it a go when I'm back.

WinDLL('./_sqlite3.dll') succeeds, which just delays the call to GetFullPathNameW to the CDLL constructor, so I don't see how the working directory is a factor. The difference I see is the lack of the LOAD_LIBRARY_SEARCH_DEFAULT_DIRS flag. Try including the individual flags (i.e. LOAD_LIBRARY_SEARCH_SYSTEM32, LOAD_LIBRARY_SEARCH_APPLICATION_DIR, LOAD_LIBRARY_SEARCH_USER_DIRS) one by one until it works.

We could enable loader snaps in the registry for the Python executable; run it as a debugger; and log the debug output to see exactly what the loader is failing to find and where it's searching.

I just wanted to acknowledge that this was breaking on my Windows 7 builder (with a bad DLL load parameter in both pythoninfo and test steps).

It looks like I was missing the required KB2533625 (the machine is mostly a virgin SP1 install), so I've installed that now.  I've restarted the most recent build and it's already past both previous error points successfully.

Windows 7 is clearly on the wane, but as there may still be other systems in a similar state as my worker, the new KB requirement for 3.8 should probably be documented with the release.

Thanks for the info, David! I guess the update isn't part of SP1. I'll add a check to the installer and update the note in What's New.

Eryk - my thought on CWD was that the new process was not starting in the correct directory, which can sometimes happen. I just started the custom buildbots with my fix, so we'll see if it's that or something else.

I attached a build of the updated installer to PR 12636 (it's too big to attach here) in case anyone can help me test. It should block right at the start if you don't have the update, or else it'll go to the usual screen.

The message just says "install SP1 and all updates" as it always has, though the log file specifically refers to the KB. I doubt enough people are going to hit this for it to be a huge problem.

Also, I think Eryk was right that Windows 8 apparently does require the additional flag. It shouldn't affect what the test is testing, so I put it in for all versions. Just waiting on the custom buildbot to start to verify that the flag is all that's necessary and not the other changes I tried.

New changeset ac19d9652799412404aef6b357a01057df34e005 by Steve Dower in branch 'master':
bpo-36085: Add additional load flag to ensure DLL loads successfully (GH-12633)
https://github.com/python/cpython/commit/ac19d9652799412404aef6b357a01057df34e005

I can help with a Win7 test of the installer, but my currently available systems are all 32-bit - any chance at a 32-bit version of the installer?

> any chance at a 32-bit version of the installer

Done (on the PR). Thanks!

Ok, I've verified that on a Win7 system with SP1 but without KB2533625 I get the expected block screen at startup.

On my worker (SP1 with KB2533625) it proceeds to the regular installation main dialog.

I'm attaching a copy of the install log in the blocking case to the PR as requested there.

Thanks, David. I looked at the log quickly and it's what I expected, so I'll merge the PR and start advertising the change. Thanks everyone!

New changeset 79da388a4016e24c4258dcc62cd0fa9dde0acb5b by Steve Dower in branch 'master':
bpo-36085: Add installer check for KB2533625 (GH-12636)
https://github.com/python/cpython/commit/79da388a4016e24c4258dcc62cd0fa9dde0acb5b

Steve, in what's new and the installer, you've referenced KB2533625 instead of KB2533623. Do we have to open a new issue for this minor search and replace fix?

Heh, and I was so sure I'd copy pasted the right number after getting it wrong so often.

Just tag it against this bug.

I have just found out that commit 2438cdf0e93 which added the winmode argument and the documentation for it disagree. Documentation states that default is zero while the real default in code is None.

I have opened PR 19167 on github to address it