| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
On Linux, threading.Thread now sets the thread name to the operating system. configure now checks if pthread_setname_np() function is available.
|
This implementation is very basic on purpose. I plan to add support for more platform in follow-up PRs.
Demo 1 (main thread): $ ./python
>>> import os
>>> pid=os.getpid()
>>> with open(f"/proc/{pid}/task/{pid}/comm") as fp: print(f"comm = {fp.read()!r}")
...
comm = 'python\n'
>>> import _thread; _thread.set_name("demo")
>>> with open(f"/proc/{pid}/task/{pid}/comm") as fp: print(f"comm = {fp.read()!r}")
...
comm = 'demo\n'Demo 2 (thread): $ ./python
>>> import threading, os, time
>>> os.getpid()
81921
>>> t=threading.Thread(target=time.sleep, args=(60,), name="sleeper")
>>> t.start()
^Z
$ cat /proc/81921/task/81927/comm
sleeperSee also a previous attempt to implement the feature: #14578 |
Sorry, something went wrong.
I changed my mind and added a private _thread._get_name() function for tests. |
Sorry, something went wrong.
|
@pitrou @encukou @serhiy-storchaka: Would you mind to review this change? It's to set the thread name in threading.Thread to the operating system. |
Sorry, something went wrong.
I modified _thread.set_name(name) to truncate name to 15 bytes on Linux. Truncating the string in threading.Thread would be more complicated since it requires to encode the string the filesystem encoding, detect the operating system (Linux), and hardcode the 15 bytes limit there. IMO it's more convenient to truncate in _thread.set_name(). |
Sorry, something went wrong.
There was a problem hiding this comment.
This looks great, thank you!
The truncation is not pretty with non-ASCII names. I guess codepoint-preserving truncation is not worth the effort, and Linux tools need to deal with thread names being arbitrary bytes.
But, we can test the edge cases, to ensure this quality-of-life enhancement doesn't start raising exceptions in working code.
Sorry, something went wrong.
|
@encukou: I addressed your reviews. Please review the updated PR. I added tests on long names and non-ASCII names. |
Sorry, something went wrong.
Refactor also tests.
|
@encukou: Maybe the "replace" error handler can be used, instead of not setting the name if the name cannot be encoded to the filesystem encoding. What do you think? |
Sorry, something went wrong.
|
You can use FS_NONASCII. You can also use TESTFN_UNDECODABLE to test that it works with arbitrary bytes and TESTFN_UNENCODABLE to test for encoding error. Is it a hard limit for the size? Is it the same on other platforms? I would prefer to use a named constant instead of magic numbers 15, 16, 17. |
Sorry, something went wrong.
Yes. Using a longer name fails with ERANGE.
It's 16 bytes on Linux and 64 bytes on macOS, so no, it's not the same.
I failed to find a public constant for these limits. For example, Darwin MAXTHREADNAMESIZE constant is private (I'm not 100% sure, but I don't have macOS so I cannot check manually, I only read the code). |
Sorry, something went wrong.
Ok, I added tests using FS_NONASCII and TESTFN_UNENCODABLE. |
Sorry, something went wrong.
Well, then hardcoding limits for known platforms is okay. We could also determine it at runtime, but this would be too complicated.
It is the content of the file, not its name. There is a flaw in this example: what encoding do you use to decode it? How do you read the name of the thread in other process with different locale? |
Sorry, something went wrong.
|
@serhiy-storchaka: I addressed your review, please review the updated PR.
open() uses the current LC_CTYPE locale encoding by default. _thread.set_name() uses the filesystem encoding.
You have the same problem with file content. It's not a new problem. IMO the Python filesystem encoding is a better choice than UTF-8 for the thread name. |
Sorry, something went wrong.
It is only a large problem if you use locale encoding for file content. If you use a fixed encoding (e.g. UTF-8) or write encoding as a metadata before writing thee encoded content, it is not a problem or a lesser problem. |
Sorry, something went wrong.
There was a problem hiding this comment.
I disagree with some decisions, but do not want to block the PR. We can fix this later.
Sorry, something went wrong.
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
| errno = rc; | ||
| return PyErr_SetFromErrno(PyExc_OSError); | ||
| } | ||
|
|
There was a problem hiding this comment.
pthread_getname_np should add a trailing NUL byte, but like everything here, that's platform-specific. I suggest being defensive here.
| name[Py_ARRAY_LENGTH(name)-1] = 0; | |
Sorry, something went wrong.
There was a problem hiding this comment.
On what platform it does not add the null byte?
Sorry, something went wrong.
There was a problem hiding this comment.
The null byte is added on all supported platforms. Before I made sure that the buffer always ended with a null byte, but @serhiy-storchaka asked me to remove it. Let's be optimistic. We can adjust the code later if needed.
Sorry, something went wrong.
There was a problem hiding this comment.
I agree, ship it :)
Or make a few more changes if you want it to be more perfect.
Sorry, something went wrong.
| size_t len = PyBytes_GET_SIZE(name_encoded); | ||
| if (len > PYTHREAD_NAME_MAXLEN) { |
There was a problem hiding this comment.
You can also inline len. It is only used here.
Sorry, something went wrong.
On Linux, threading.Thread now sets the thread name to the operating system. * configure now checks if pthread_getname_np() and pthread_setname_np() functions are available. * Add PYTHREAD_NAME_MAXLEN macro. * Add _thread._NAME_MAXLEN constant for test_threading. Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
I am sorry I didn't get to this earlier; on Solaris, the tests are green and all works as expected. There is one small issue with Solaris detection in test_set_name and I opened #132012 to resolve that. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
On Linux, threading.Thread now sets the thread name to the operating system.
configure now checks if pthread_setname_np() function is available.