Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

platform module functions execute for a long time if user has no permissions to WMI #112278

Closed
kfrydel opened this issue Nov 20, 2023 · 14 comments
Closed
Labels
3.12 bugs and security fixes 3.13 new features, bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error

Comments

@kfrydel
Copy link

kfrydel commented Nov 20, 2023

Bug report

Bug description:

From Python 3.12, probably since #89545 was implemented, the invocation of functions from platform modules takes a multiple of 5 seconds if the user has no permissions to perform WMI queries. It can be reproduced by connecting to a Windows machine using SSH and executing a Python script from cmd or ps. Access to WMI for remote sessions is turned off by default but this probably can be achieved also by removing permissions using wmimgmt.msc application.

For example, the execution of the system function in Python 3.12:

user@WIN16 c:\Python3.12>python.exe -m timeit -s "import platform" -n 1 -r 1 "platform.system()"                       
1 loop, best of 1: 10.2 sec per loop

and the same in Python 3.11:

user@WIN16 c:\Python3.11>python.exe -m timeit -s "import platform" -n 1 -r 1 "platform.system()"                       
1 loop, best of 1: 18.1 msec per loop    

(timeit used with -n 1 -r 1 to avoid using cache in the platform module. When results get cached then of course it works fast).

The delay comes from de33df2#diff-cb1ba6039236dca71c97ea898f2798c60262c30614192862259889f839e4d503R323 which takes 5 seconds and raises OSError: [WinError -2147217405] Windows Error 0x80041003. platform.system apparently calls it twice. This applies to all functions from the module that makes WMI queries.

Windows version used for tests:
Edition: Windows Server 2016 Standard
Version: 1607
OS Build: 14393.6452

CPython versions tested on:

3.12

Operating systems tested on:

Windows

Linked PRs

@kfrydel kfrydel added the type-bug An unexpected behavior, bug, or error label Nov 20, 2023
@kfrydel
Copy link
Author

kfrydel commented Nov 21, 2023

It's very hard to work around this or avoid the delay. Even if you do not use platform in your code, or use sys/os replacements like os.name or sys.getwindowsversion() it sooner or later turns out that the platform module is used by some 3rd party library you import because it is widely used in Python's world.

@gozdal
Copy link

gozdal commented Nov 22, 2023

As an example, merely importing aiohttp or SQLAlchemy in a session over SSH on Windows Server 2019 using Python 3.12:

Python 3.12.0 (tags/v3.12.0:0fb18b0, Oct  2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import timeit
>>> timeit.timeit('import aiohttp', number=1)
10.157976499991491
Python 3.12.0 (tags/v3.12.0:0fb18b0, Oct  2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import timeit
>>> timeit.timeit('import sqlalchemy', number=1)
10.204078600043431

cc @zooba as the original author

@radkujawa
Copy link

bumping this, it is a serious issue in my project

@aisk
Copy link
Member

aisk commented Dec 3, 2023

I think we can replace the platform._wmi_query with the dummy implementation when first failure, this can reduce the time costs to nearly 5 seconds, (just one timeout when connect to WMI):

PS C:\Users\xxxxx\Source\cpython> .\python.bat -m timeit -s "import platform" -n 1 -r 1 "platform.system()"
Running Debug|x64 interpreter...
1 loop, best of 1: 5.12 sec per loop

Then I think we should set the timeout for the connection to WMI. But https://learn.microsoft.com/en-us/windows/win32/api/wbemcli/nf-wbemcli-iwbemlocator-connectserver said that the timeout can only be set to "2 minutes or less" and there seems to no other way to set it to a custom value. So using a Event object to simulate the timeout may be a solution.

@zooba
Copy link
Member

zooba commented Dec 7, 2023

This should be backported to 3.12 as well, but they're right in the middle of getting 3.12.1 ready and I don't think this is that urgent. Monkeypatch platform._wmi_query early in your app if you need to suppress it.

@kfrydel
Copy link
Author

kfrydel commented Dec 7, 2023

Thank you for fixing this! For 3.12.1 we will use the runtime hooks feature from pyinstaller to override the _wmi_query function before anything else is run.

@zooba
Copy link
Member

zooba commented Dec 8, 2023

The timeout is too short - I'm getting failures on some of my test machines. It wouldn't surprise me if it's due to them being slower, and so COM takes longer to load and initialize on its first use.

We can either make the timeout longer (I'd suggest 1 second), or add a second event that sets just before the connection attempt. That way we can wait for everything to load with a long timeout, but only wait for the connection with a short timeout.

@aisk
Copy link
Member

aisk commented Dec 8, 2023

The timeout is too short - I'm getting failures on some of my test machines. It wouldn't surprise me if it's due to them being slower, and so COM takes longer to load and initialize on its first use.

We can either make the timeout longer (I'd suggest 1 second), or add a second event that sets just before the connection attempt. That way we can wait for everything to load with a long timeout, but only wait for the connection with a short timeout.

Increasing the timeout to 1 second will lead to _wmi_query taking more than 1 second to call without WMI permission, even if the COM has loaded and initialized. Therefore, I think the second approach is better.

@zooba
Copy link
Member

zooba commented Dec 8, 2023

Thanks for the PR! That makes the backport a bit more complicated, but I think once we've seen it's stable in 3.13 then we can just backport the entire file - nothing else should've changed here for 3.13.

@zooba zooba added 3.12 bugs and security fixes 3.13 new features, bugs and security fixes labels Dec 11, 2023
@zooba
Copy link
Member

zooba commented Dec 14, 2023

We still have timeouts occurring in test runs where they previously passed. Probably just need to add a sleep and a retry in the tests, since it ought to pass. If we add a test configuration where it shouldn't, then we can skip the entire test.

@aisk
Copy link
Member

aisk commented Dec 14, 2023

If the tests timeout in the tests, I think it should also timeout in users' code. Can we just increase the timeout for initEvent to avoid it?

@zooba
Copy link
Member

zooba commented Dec 14, 2023

Increasing the timeout doesn't bother me, but no guarantee that it'll actually fix it. We can't tell right now whether it's slow to initialise or slow to connect (having separate error messages might help there too, but it'll make the code way more complicated).

@aisk
Copy link
Member

aisk commented Dec 15, 2023

Got it, added retry and sleep in the WMI tests.

@zooba
Copy link
Member

zooba commented Apr 12, 2024

If the fix in #117818 reduces/fixes the occasional errors we've been getting in CI, then I'll consider this issue complete and close it.

Reviews/manual testing appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 new features, bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

6 participants