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

Handle STATUS_INVALID_INFO_CLASS in psutil_proc_exe #2493

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

klightspeed
Copy link

@klightspeed klightspeed commented Jan 1, 2025

Summary

Description

Wine versions before 9.8 and Proton versions before 9.0-1 did not implement the SystemProcessIdInformation system information class in NtQuerySystemInformation, returning STATUS_INVALID_INFO_CLASS.

Ubuntu 24.04.1 LTS currently ships with Wine 9.0, and CrossOver 24.0.5 has not backported the implementation of the SystemProcessIdInformation system information class to its bundled Wine 9.0

Handle this error case and fall back to QueryFullProcessImageNameW to get the process name.

@klightspeed klightspeed force-pushed the proc_exe-handle-invalid-param branch from aadac57 to 9fa8c73 Compare January 1, 2025 09:18
@giampaolo
Copy link
Owner

giampaolo commented Jan 1, 2025

If my memory serves me right Wine is not supported, meaning attempting to compile resulted in many errors due to missing Windows APIs. Does this mean that with this PR all errors are fixed and compilation on Wine succeeds?

Did it succeed before?

Wine versions before 9.8 and Proton versions before 9.0-1 did not
implement the `SystemProcessIdInformation` system information class in
`NtQuerySystemInformation`, returning `STATUS_INVALID_INFO_CLASS`.

Ubuntu 24.04.1 LTS currently ships with Wine 9.0, and CrossOver
24.0.5 has not backported the implementation of the
`SystemProcessIdInformation` system information class to its bundled
Wine 9.0

Handle this error case and fall back to `QueryFullProcessImageNameW` to
get the process name.

Signed-off-by: Ben Peddell <[email protected]>
@klightspeed klightspeed force-pushed the proc_exe-handle-invalid-param branch from 9fa8c73 to db39d01 Compare January 1, 2025 12:57
@klightspeed klightspeed changed the title Handle STATUS_INVALID_PARAMETER in psutil_proc_exe Handle STATUS_INVALID_INFO_CLASS in psutil_proc_exe Jan 1, 2025
@klightspeed
Copy link
Author

I wasn't able to install the Visual C++ build tools in my wine prefix, but building on a Windows machine then deploying the wheel to the Wine prefix did work.

$ wine --version
wine-9.0 (Ubuntu 9.0~repack-4build3)
$ cd ~/.wine/drive_c
$ cp ~/psutil-7.0.0-cp37-abi3-win_amd64.whl .
$ wine cmd
Microsoft Windows 10.0.19043

C:\>python --version
Python 3.12.8

C:\>mkdir test-psutil

C:\>cd test-psutil

C:\test-psutil>python -m venv .

C:\test-psutil>Scripts/activate.bat

(test-psutil) C:\test-psutil>pip install ..\psutil-7.0.0-cp37-abi3-win_amd64.whl
Processing c:\psutil-7.0.0-cp37-abi3-win_amd64.whl
Installing collected packages: psutil
Successfully installed psutil-7.0.0

(test-psutil) C:\test-psutil>python
Python 3.12.8 (tags/v3.12.8:2dc476b, Dec  3 2024, 19:30:04) [MSC v.1942 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import psutil
>>> psutil.pids()
[56, 68, 76, 112, 168, 192, 224, 1088, 1096, 1436, 1444, 1452, 1576, 1584]
>>> psutil.Process(1584)
psutil.Process(pid=1584, name='python.exe', status='running', started='13:15:49')
>>> quit()

@klightspeed klightspeed marked this pull request as ready for review January 1, 2025 13:51
@klightspeed
Copy link
Author

klightspeed commented Jan 1, 2025

If my memory serves me right Wine is not supported, meaning attempting to compile resulted in many errors due to missing Windows APIs.

After a fair bit of wrangling, I was able to install the Visual Studio 2019 Build Tools in Wine (with the Visual Studio installer being very temperamental), install Python in Wine, set up the right environment variables (frustrated by the fact that cmd.exe in Wine always pretends to be 32-bit), and build the package.

Basically:

$ cd ~
$ mkdir wine-psutil && cd wine-psutil
$ export WINEARCH=win32
$ export WINEPREFIX="$PWD/wine"
$ wineboot -i
$ winetricks corefonts d3dcompiler_47 dotnet472 # requires UI interaction
$ winetricks vstools2019 # requires UI interaction
$ winecfg -v win10
$ wget https://www.python.org/ftp/python/3.11.9/python-3.11.9.exe
$ wine python-3.11.9.exe # requires UI interaction
$ git clone https://github.com/giampaolo/psutil.git && cd psutil
$ wine cmd
> "C:\Program Files\Microsoft Visual Studio\2019\BuildTools\VC\Auxiliary\Build\vcvars32.bat"
> mkdir venv
> python -m venv venv
> venv\Scripts\activate.bat
> python -m pip install --upgrade pip
> pip install setuptools wheel
> set DISTUTILS_USE_SDK=1
> make.bat build
> make.bat wheel
> pip install dist\psutil-7.0.0-cp37-abi3-win32.whl

Does this mean that with this PR all errors are fixed and compilation on Wine succeeds?
Did it succeed before?

This PR does not change the build environment.

@giampaolo
Copy link
Owner

Can you paste the output of make.bat build? (but do make.bat clean first)

@giampaolo
Copy link
Owner

giampaolo commented Jan 2, 2025

...and if it's not too much trouble, could you also paste the output of unit tests (make.bat test)?
I'm curious to see what works and what doesn't.

NtQuerySystemInformation is used also in other places, so I expect psutil compiles on Wine but it's not 100% working. (still, it's a lot better than a though :D)

@klightspeed
Copy link
Author

The tests die on psutil/tests/test_process.py::TestProcess::test_open_files, and on the wine64 prefix I managed to get the Visual C++ Build Tools installed on, it also occasionally dies on psutil/tests/test_misc.py::TestMisc::test_process_as_dict_no_new_names
Windows 11: make-test-win11.txt
Wine 9.0 on master: test-wine-master.txt
Wine 9.0 on this PR: test-wine-PR2493.txt

I am currently bringing up a Wine 9.17 win32 prefix to test.

@klightspeed
Copy link
Author

klightspeed commented Jan 3, 2025

The exe test breaks on newer Wine versions when run from a drive other than the one that Python is installed on due to the executable path not including the drive letter. Looks like I need to submit a bug report to them that SystemProcessIdInformation doesn't include the drive letter. Correction: convert_dos_path does not handle NT paths of the form \??\X:

>python
Python 3.11.9 (tags/v3.11.9:de54cf5, Apr  2 2024, 10:00:00) [MSC v.1938 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import psutil
>>> psutil.pids()
[32, 56, 68, 80, 100, 132, 192, 244, 280, 288, 1980, 1988]
>>> p = psutil.Process(1988)
>>> p
psutil.Process(pid=1988, name='python.exe', status='running', started='09:44:01')
>>> p.exe
<bound method Process.exe of psutil.Process(pid=1988, name='python.exe', status='running', started='09:44:01')>
>>> p.exe()
'\\Program Files\\Python311-32\\python.exe'

test-wine9.17-master.txt

@giampaolo
Copy link
Owner

giampaolo commented Jan 3, 2025

The tests die on psutil/tests/test_process.py::TestProcess::test_open_files.

You can try returning an empty python list here, de-facto disabling Process.open_files() on Wine (which you have to detect at compile time with #ifdef WINE or something - not sure):
https://github.com/giampaolo/psutil/blob/master/psutil/arch/windows/proc_handles.c#L212
Process.open_files() has historically been a pain, so I don't think it's worth the effort to try to make it work on Wine.

For clarity: I'm still trying to understand whether it's worth supporting Wine at all. If it's just these 2 changes we can go for it.

@giampaolo
Copy link
Owner

...and this is the (old) issue where Wine support was being discussed: #1448.

@klightspeed
Copy link
Author

The tests die on psutil/tests/test_process.py::TestProcess::test_open_files.

You can try returning an empty python list here, de-facto disabling Process.open_files() on Wine (which you have to detect at compile time with #ifdef WINE or something - not sure): https://github.com/giampaolo/psutil/blob/master/psutil/arch/windows/proc_handles.c#L212 Process.open_files() has historically been a pain, so I don't think it's worth the effort to try to make it work on Wine.

For clarity: I'm still trying to understand whether it's worth supporting Wine at all. If it's just these 2 changes we can go for it.

After putting in runtime detection for Wine (detecting whether the wine_get_version function is exported from ntdll.dll), here are the tests with Wine 9.22:
make-test-wine-bypass-open_files.txt
And with 9.0:
make-test-wine-bypass-open_files-9.0.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Wine] NtQuerySystemInformation(SystemProcessIdInformation) not implemented before Wine 9.8
2 participants