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

[POSIX] psutil.users() loses precision for "started" attribute #2225

Closed
djast opened this issue Apr 10, 2023 · 8 comments · Fixed by #2226
Closed

[POSIX] psutil.users() loses precision for "started" attribute #2225

djast opened this issue Apr 10, 2023 · 8 comments · Fixed by #2226
Labels

Comments

@djast
Copy link

djast commented Apr 10, 2023

In _psutil_linux.c, psutil_users() casts ut->ut_tv.tv_sec to type float (single-precision); this is insufficient to represent contemporary timestamp values with reasonable accuracy.

Currently, psutil.users()[i].started can be inaccurate by more than a full minute.

Note that while Python float values are represented as 64-bit double-precision values, the float type in C is typically only 32-bit. The code should probably use "000d0" _Py_PARSE_PID and (double)ut->ut_tv.tv_sec rather than "000f0" _Py_PARSE_PID and (float)ut->ut_tv.tv_sec in the Py_BuildValue() call.

@github-actions github-actions bot added the linux label Apr 10, 2023
@giampaolo
Copy link
Owner

Please create a PR or a diff to attach to this ticket.

@djast
Copy link
Author

djast commented Apr 10, 2023

I don't have a lot of CPython dev experience, but I think this is what's needed for the Linux case:

psutil.txt

However, a similar fix is almost certainly needed for (at least) _psutil_aix.c, _psutil_bsd.c, _psutil_osx.c, and _psutil_sunos.c; some of these may also need comparable changes in their psutil_boot_time() functions (which appear to use utmp rather than /proc/stat as Linux does).

In a nutshell, anywhere the code says "(float)ut->ut_tv.tv_sec" or "(float)utx->ut_tv.tv_sec" requires attention.

@giampaolo
Copy link
Owner

Yes, I added a unit test and I could verify that the time is off by 1 minute compared to who command.

@giampaolo
Copy link
Owner

PR: #2226.

@djast
Copy link
Author

djast commented Apr 10, 2023

"boot_time = (float)ut->ut_tv.tv_sec" in _psutil_aix.c and _psutil_sunos.c probably also need similar fixes (the declaration of the boot_time variable, the cast, and the Py_BuildValue call).

@giampaolo
Copy link
Owner

Yes, I have changed it also on those platforms.

@djast
Copy link
Author

djast commented Apr 11, 2023

I see that you fixed psutil_users() for those platforms; I'm referring to psutil_boot_time(), which appears to have a nearly identical bug:

boot_time = (float)ut->ut_tv.tv_sec;

boot_time = (float)ut->ut_tv.tv_sec;

Technically these bugs aren't related to this psutils.users() issue, but assuming the code is used, it appears to me that psutil.boot_time() would have a similar loss of precision. (I'd file this as a separate issue, but I don't have those platforms available to confirm the bug.)

@giampaolo giampaolo changed the title [Linux] psutil.users() loses precision for "started" attribute [POSIX] psutil.users() loses precision for "started" attribute Apr 11, 2023
@giampaolo giampaolo added unix and removed linux labels Apr 11, 2023
@giampaolo
Copy link
Owner

Thank you. I have created #2228 to keep track of the boot_time() precision issue.

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

Successfully merging a pull request may close this issue.

2 participants