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

thread names : linux and windows support #1559

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

romainpereira
Copy link

@romainpereira romainpereira commented Jul 31, 2019

This patch adds support for thread names on Linux and Windows, by adding an attribute name within the tuple returned by psutil.threads()
On other operating system, the string is empty

The code is probably over-commented, we can clean this.

1 - Shall we remove the MacOSX comments, or add them as a comment of this PR?

2 - Remove the thread-names-demo/ folder, which is only here giving some small sample program to be tested

3 - Here is another way of naming thread in Windows, to have thread names appearing in Visual Studio debugger:
https://blogs.msdn.microsoft.com/stevejs/2005/12/20/naming-threads-in-win32-and-net/
I think the debugger see the exception while reading process's task, and interpret the exception message as the thread name.

The SetThreadDescription() call is really likely to be used to name threads now.

4 - Do we really reject the use of unicode string? C.f psutil/tests/test_contracts.py:194
We probably want them on the name field, since Windows thread names are wide characters string.


Please, note that a previous thread name support implementation was tried.
This patch is originally inspired by this pull-request: #1135
on which both UbiCastTeam and fthiery worked.

@romainpereira
Copy link
Author

romainpereira commented Jul 31, 2019

Sorry for various failures on CIs, got some false positive locally, that's why.
Issues are fixed now.

Still, I do not know whats going on with 2.7 and 3.5 on appveyor,
and why Python 2.7 travis build failed on 2998.6 , but not on 2997.6

Any ways to retrigger build without making a new commit?

@rpereira-dev
Copy link

@giampaolo I allow myself to ping you, since no one reacted in 2 months

Did you give a look to the patch?
Do you know what's making the CI (appveyor) failing for Python 2.7 and 3.5?

Thank you very much!

@giampaolo
Copy link
Owner

Hello. No I didn’t have time to look at the patch (and I will still be busy for a while), but in principle I agree it would be good to have thread names. If in the meantime you could take a look at BSD and OSX implementation that would be great.

@rpereira-dev
Copy link

rpereira-dev commented Sep 28, 2019

I no longer have access to a OSX machine :(
But as far as I dig last month, there is no 'easy' way (i.e, a syscall) to access the thread names of any process on OSX (as GetThreadDescription() system call on Windows, or reading in /proc/... on Linux)

See my comments on psutil/_psutil_osx.c

@giampaolo
Copy link
Owner

Can you please remove the demos and other unnecessary stuff?

@rpereira-dev
Copy link

rpereira-dev commented Nov 21, 2019

Can you please remove the demos and other unnecessary stuff?

Done. Not sure about what's making the CI fail though

@@ -1914,7 +1913,12 @@ def threads(self):
values = st.split(b' ')
utime = float(values[11]) / CLOCK_TICKS
stime = float(values[12]) / CLOCK_TICKS
ntuple = _common.pthread(int(thread_id), utime, stime)
try:
with open_binary("%s/comm" % tdir) as fcomm:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you shoud use open_text instead

}
get_thread_desc = (get_thread_desc_t) GetProcAddress(
GetModuleHandle("Kernel32.dll"), "GetThreadDescription"
);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff should be done differently.
You should first define the prototype of GetThreadDescription in psutil/arch/windows/ntext.h then load GetThreadDescription and set it as a global name in psutil/_psutil_common.c.

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

Successfully merging this pull request may close these issues.

None yet

3 participants