Threading: add builtin TID attribute to Thread objects #80265
Comments
|
This functionality adds a native Thread ID to threading.Thread objects. This ID (TID), similar to the PID of a process, is assigned by the OS (kernel) and is generally used for externally monitoring resources consumed by the running thread (or process). |
jaketesler
mannequin
added
3.8 (EOL)
|
*bump* Could someone look into reviewing this bug/PR? Thanks! |
|
It seems like the feature is only supported by a few operating systems and only if required functions are available: #ifdef __APPLE__
volatile uint64_t tid;
pthread_threadid_np(NULL, &tid);
#elif defined(__linux__)
volatile pid_t tid;
tid = syscall(__NR_gettid);I understand that it's not available on FreeBSD nor Windows? In that case, I would prefer to only add a threading.get_tid() *function*. Is it different than threading.get_ident() on Linux? |
|
The feature is supported on Windows: the file supporting Windows threading is Also it is different from threading.get_ident() - ident is a Python-issued unique identifier, TID is issued by the OS and is tracable system-wide. |
|
Thank you Jake for your contribution! |
|
I dislike adding a function which always return 0 when the feature is not supported: unsigned long
PyThread_get_thread_native_id(void)
{
...
#if ...
...
#else
unsigned long native_id;
native_id = 0;
#endif
return (unsigned long) native_id;
}I would prefer to not define the function if it's not available, so the attribute would be missing. With the commited change, how can I know if native thread identifier is supported or not? |
Why not set the attribute to None before a thread starts? It would be more consistent with the the "ident" attribute behavior, no? Extract __repr__(): def __repr__(self):
assert self._initialized, "Thread.__init__() was not called"
status = "initial"
if self._started.is_set():
status = "started"
...
if self._ident is not None:
status += " %s" % self._ident
return "<%s(%s, %s)>" % (self.__class__.__name__, self._name, status)"is None" is a reliable check to decide if the attribute is set or not. With the current implementation, it's hard to guess since PyThread_get_thread_native_id() returns on some platforms... But again, I would prefer to not have the attribute if PyThread_get_thread_native_id() is not available on a platform. |
|
This change broke the AIX buildbot: ====================================================================== Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_threading.py", line 109, in test_various_ops
self.assertEqual(len(native_ids), NUMTASKS + 1)
AssertionError: 1 != 11====================================================================== Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_threading.py", line 159, in test_various_ops_large_stack
self.test_various_ops()
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_threading.py", line 109, in test_various_ops
self.assertEqual(len(native_ids), NUMTASKS + 1)
AssertionError: 1 != 11====================================================================== Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_threading.py", line 147, in test_various_ops_small_stack
self.test_various_ops()
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_threading.py", line 109, in test_various_ops
self.assertEqual(len(native_ids), NUMTASKS + 1)
AssertionError: 1 != 11 |
|
So where do we go from here? |
I propose to only add attribute if it's supported. If nobody comes with a fix, I would prefer to remove the feature to repair the AIX buildbot. |
|
I wrote PR 13458 to revert the change which broke the CI for longer than one week, see the rationale there: Email thread about the regression: |
|
Jake Tesler: In the meanwhile, can you please try to rewrite your change to make the attribute optional? C PyThread_get_thread_native_id() function and Python _thread.get_native_id() should not be defined if it's not supported by I suggest to add the following #define in Include/pythread.h: #if (... list of supported platforms ...
# define PY_HAVE_THREAD_NATIVE_ID 1
#endifIt would be great if you can come up with a solution before the end of the month, otherwise we will miss Python 3.8 deadline for new feature :-( Tell me if you need help to rework your PR. IMHO it's a nice feature. There is just an issue in your first implementation. |
|
Jake, would you like to submit a new PR that implements Victor's suggestion (i.e. only define and test the attribute on supported systems)? |
|
I will implement these changes - let’s try to hit that end-of-month target! |
|
New PR created with requested edits. :) |
|
Victor Stinner: would you mind taking a look at the new PR? Is this more along the lines of what you had in mind? |
|
Repeating a bot of what I added to PR13463 AIX has native support for thread-id since at least AIX 4.1 (1994-1995) where every process has an initial TID (PID are even numbers and "own" the resources, TID are odd and are the "workers" - very simply put). Hence, by default, an AIX process is "mono-threaded". The key concern - when calling thread related issues, is to ensure that all compiled code uses the same definitions in include files - namely, with _THREAD_SAFE defined. There are couple of ways to accomplish this: As a) seems unpractical to ensure all the time; b) is also unpractical (no idea how/if gcc, e.g., has a way to 'signal' the need to be thread_safe - so a change in configure.ac, and maybe in setup.py, and thinking further yet - in the CFLAGS that get passed to extrnal modules and extensions - adding -D_THREAD_SAFE seems the most complete (aka safe) approach. And, of course, for this specific function a call to Syntax
#include <pthread.h>
pthread_t pthread_self (void);Description |
|
The AIX case sounds way more complex. I suggest to start without AIX |
|
I do not know if it is that much mode complex. Unless I missed something it seems to be that this bit - needs three lines added after the FREEBSD block - per below: #ifdef PY_HAVE_THREAD_NATIVE_ID
unsigned long
PyThread_get_thread_native_id(void)
{
if (!initialized)
PyThread_init_thread();
#ifdef __APPLE__
uint64_t native_id;
(void) pthread_threadid_np(NULL, &native_id);
#elif defined(__linux__)
pid_t native_id;
native_id = syscall(SYS_gettid);
#elif defined(__FreeBSD__)
int native_id;
native_id = pthread_getthreadid_np();
#elif defined(_AIX)
pthread_t native_id;
native_id = pthread_self()More may be needed, but I expect it the include file mentioned is already included - but perhaps without the assurance that AIX says it wants/needs for real thread safe builds. And fixing that is just a bonus! |
|
On 22/05/2019 12:22, Michael Felt wrote:
Now that I think about it, perhaps I should make a separate PR just for |
I don't know much about AIX, but according to the docs 1 the kernel thread ID should be thread_self(), which is not the same as pthread_self(). |
|
In general, I’ve concluded most ‘editions’ of pthread_self() are not the same value as this feature aims to implement. I’m not familiar enough with AIX to be certain about that platform, though. If there’s an equivalent function in AIX to capture the actual integral thread ID in the same 3-line way as Linux, FreeBSD, et al., are implemented (the big block in PyThread_get_thread_native_id), then it’s worth including. If the mechanism required is a more complex one than just adding a few lines, then it might be best left for a new PR. :) |
|
I will look into whether adding thread_self() for AIX would be simple enough for this PR. |
|
Currently, Threading.start() ignores _start_new_thread() return value which is an identifier. Is it the same value than threading.get_native_id()? It might be good to clarify _thread._start_new_thread() documentation since we now have 2 kinds of "identifier". Later it might be interesting to attempt to support NetBSD, OpenBSD, and more operating systems. Well, that can be done... later :-) |
|
glibc 2.30 scheduled in August 2019 will finally provide a gettid() function!
Once it will be released, it would be interesting to use it rather than using a direct syscall. |
|
On 22/05/2019 15:15, Jake Tesler wrote:
Blush. Maybe I should have read further (to chapter T) Here is a bare bones example - showing the pthread_self() is not the michael@x071:[/data/prj/aixtools/tests/posix]cat thread*.c
#include <pthread.h>
#include <sys/thread.h>
#include <stdio.h>main()
{
pid_t pid;
tid_t tid;
pthread_t ptid; pid = getpid(); fprintf(stderr,"thread_self: pid:%d tid:%d ptid:%d\n", pid, tid, michael@x071:[/data/prj/aixtools/tests/posix]./thread_self michael@x071:[/data/prj/aixtools/tests/posix]ps -mo THREAD |
|
Michael Felt: it's annoying when you ignore Antoine's comment and my comment.
The AIX case is very special and required a separated issue. Please open a separated if you want to discuss/implement get_native_id() on AIX. |
|
Victor – the return value of _start_new_thread is the the See here: #11993 (comment) |
|
On 22/05/2019 18:08, STINNER Victor wrote:
My apologies. I was not ignoring anyone. I am sorry you had that impression. I had already taken it as a given that it would not be in this PR (re: And, I had expressed my hope - it would not be too complex in And, while I am sorry you feel I have ignored you - it is hardly the Thank you for your honesty!
|
|
Michael Felt - |
|
If someone wants to document that _start_new_thread() return value is similar to threading.get_ident() but not threading.get_native_id(): please go ahead and propose a PR :-) Adding support for AIX to get_native_id() should be done in a separated issue since AIX seems to come with specific challenges :-) The initial feature request is now implemented. Well done! I know that many people were awaiting to expose "gettid()" in Python. Now it's there! Thanks everyone who was involved in this issue. Sorry for the CI hiccup which required to revert the change temporarily. At least, it didn't miss Python 3.8 feature freeze! |
|
On 23/05/2019 18:16, Jake Tesler wrote:
|
|
I have encountered a minor bug with this new feature. The bug occurs when creating a new multiprocessing.Process object on Unix (or on any platform where the multiprocessing start_method is 'fork' or 'forkserver'). When creating a new process via fork, the Native ID in the new MainThread is incorrect. The new forked process' threading.MainThread object inherits the Native ID from the parent process' MainThread instead of capturing/updating its own (new) Native ID. See the following snippet: >>> import threading, multiprocessing
>>> multiprocessing.set_start_method('fork') # or 'forkserver'
>>> def proc(): print(threading.get_native_id(), threading.main_thread().native_id) # get_native_id(), mainthread.native_id
>>> proc()
22605 22605 # get_native_id(), mainthread.native_id
>>> p = multiprocessing.Process(target=proc)
>>> p.start()
22648 22605 # get_native_id(), mainthread.native_id
>>>
>>> def update(): threading.main_thread()._set_native_id()
>>> def print_and_update(): proc(); update(); proc()
>>> print_and_update()
22605 22605 # get_native_id(), mainthread.native_id
22605 22605
>>> p2=multiprocessing.Process(target=print_and_update); p2.start()
22724 22605 # get_native_id(), mainthread.native_id
22724 22724
>>> print_and_update()
22605 22605 # get_native_id(), mainthread.native_id
22605 22605As you can see, the new Process object's MainThread.native_id attribute matches that of the MainThread of its parent process. Unfortunately, I'm not too familiar with the underlying mechanisms that Multiprocessing uses to create forked processes. I've created a branch containing a working fix - I'm also open to suggestions of how a fix might otherwise be implemented. See the branch here: https://github.com/jaketesler/cpython/tree/fix-mp-native-id Thanks all! |
Please open a new issue. |
|
Jake created bpo-38707. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: