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

bpo-36084: add native thread ID (TID) to threading.Thread objects (V2) #13463

Merged
merged 28 commits into from
May 22, 2019

Conversation

jaketesler
Copy link
Contributor

@jaketesler jaketesler commented May 21, 2019

This is the second version of PR #11993 (original reverted due to test failures on unsupported operating systems).

cc @vstinner @pitrou

From the original PR:

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).
This does not replace the ident attribute within Thread objects, which is assigned by the Python interpreter and is guaranteed as unique for the lifetime of the Python instance.

This new functionality fully supports Linux, Apple, and Windows platforms.

https://bugs.python.org/issue36084

jaketesler and others added 22 commits February 22, 2019 09:55
…jects now have a tid property (kernel thread ID)
…c operation, since the functionality now supports Windows as well; cleaned up and clarified doc notes, updated version added number (3.8)
… ID functions depending on system type and functional availability; Threading module now returns `None` if the thread has not been started, or if the Native ID functionality does not exist for the current operating system.
Lib/_dummy_thread.py Outdated Show resolved Hide resolved
Lib/test/test_threading.py Outdated Show resolved Hide resolved
Lib/threading.py Outdated Show resolved Hide resolved
Lib/threading.py Outdated Show resolved Hide resolved
Python/thread_pthread.h Outdated Show resolved Hide resolved
Python/thread_pthread.h Show resolved Hide resolved
@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@jaketesler
Copy link
Contributor Author

Per @bedevere-bot
I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

Python/thread_nt.h Outdated Show resolved Hide resolved
Doc/library/_thread.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Python/thread_pthread.h Show resolved Hide resolved
Python/thread_pthread.h Show resolved Hide resolved
Python/thread_pthread.h Outdated Show resolved Hide resolved
Python/thread_pthread.h Outdated Show resolved Hide resolved
Python/thread_pthread.h Show resolved Hide resolved
@jaketesler
Copy link
Contributor Author

@vstinner please advise if there are any additional suggestions you have or changes that should be implemented before the merge.

@aixtools
Copy link
Contributor

This is the second version of PR #11993 (original reverted due to test failures on unsupported operating systems).

cc @vstinner @pitrou

From the original PR:

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).
This does not replace the ident attribute within Thread objects, which is assigned by the Python interpreter and is guaranteed as unique for the lifetime of the Python instance.
This new functionality fully supports Linux, Apple, and Windows platforms.

I expect the reason it broke AIX - is because AIX supports - for nearly 30 years is my guess (not known if it was already in AIX 3.X, but always in AIX 4.X).
From the AIX 6.1 documentation:

pthread_self Subroutine
Purpose
Returns the calling thread's ID.
Library
Threads Library (libpthreads.a)
Syntax
#include <pthread.h>
pthread_t pthread_self (void);
Description
The pthread_self subroutine returns the calling thread's ID.
Note: The pthread.h header file must be the first included file of each source file using the threads
library. Otherwise, the -D_THREAD_SAFE compilation flag should be used, or the cc_r compiler
used. In this case, the flag is automatically set.
Return Values
The calling thread's ID is returned.
Errors
No errors are defined.
The pthread_self function will not return an error code of EINTR.

The 'key' issue being that <pthread.h> needs to be first (or -D_THREAD_SAFE, which may be 'better' to ensure that all C-code is compiled "thread-safe" when using gcc. I normally use xlc_r, which sets _THREAD_SAFE - but I cannot assume that gcc (which is what the AIX bots currently use) are compiling everything "thread-safe".
Iirc, among other things, "thread-safe" means the linker links with the "thread_safe" libraries (provided by IBM. No guarantee that third-party libraries are thread-safe. etc..

Anyway, moving forward - how can I best contribute some code specific to AIX. Never tried working "together" before.

@jaketesler
Copy link
Contributor Author

To be clear, in all the POSIX-based platforms that I’ve worked with to build this feature (notably excluding AIX, which I have zero experience with), *_self()-type functions are NOT the same value/ID as the one used by this feature.

Can you confirm whether or not AIX has the capability to return the integral ID, or whether the pthread_self() function simply returns the same struct reference as other POSIX systems? Given the return type pthread_t, at first glance I’m inclined to believe it’s not the same functionality.

@jaketesler
Copy link
Contributor Author

Looking at docs, I think this might be what we’re looking for: https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.basetrf2/thread_self.htm

I will look into this and see if it’d be easy to add for this PR.

@pitrou
Copy link
Member

pitrou commented May 22, 2019

I'd recommend tackling AIX as a separate PR.

@jaketesler
Copy link
Contributor Author

Either way :)

In that case, I think this is mostly good to go (pending review of course).

@vstinner
Copy link
Member

I could continue to nitpick to death, but well, you addressed my main concern: not add the function/attribute on unsupported platforms, so I approve your change. Thanks for fixing it!

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.

6 participants