From 51d657968f014cd0f661397658d37ba23221679f Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Thu, 20 Feb 2025 23:33:45 -0600 Subject: [PATCH 1/6] Return the main thread's short identifier on certain platforms CPython's pthread-based thread identifier relies on pthread_t being able to be represented as an unsigned integer type. This is true in most Linux libc implementations where it's defined as an unsigned long, however musl typedefs it as a struct *. If the pointer has the high bit set and is cast to PyThread_ident_t, the resultant value can be sign-extended [0]. This can cause issues when comparing against threading._MainThread's identifier. The main thread's identifier value is retrieved via _get_main_thread_ident which is backed by an unsigned long which truncates sign extended bits. >>> hex(threading.main_thread().ident) '0xb6f33f3c' >>> hex(threading.current_thread().ident) '0xffffffffb6f33f3c' Work around this by conditionally compiling in some code for non-glibc based Linux platforms that are at risk of sign-extension to return a PyLong based on the main thread's unsigned long thread identifier if the current thread is the main thread. [0]: https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Arrays-and-pointers-implementation.html Signed-off-by: Vincent Fazio --- Modules/_threadmodule.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index e251736fb36aa9..65f504253ba3ea 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -2029,6 +2029,13 @@ An obsolete synonym of allocate_lock()."); static PyObject * thread_get_ident(PyObject *self, PyObject *Py_UNUSED(ignored)) { + /* Work around an issue with the main thread ID to failing comparison checks + due to sign extension on some Linux libc implemenations. Can be removed + when thread identifiers are reworked. */ +#if SIZEOF_LONG < SIZEOF_LONG_LONG && defined(__linux__) && !defined(__GLIBC__) + if (_Py_IsMainThread()) + return PyLong_FromUnsignedLong(_PyRuntime.main_thread); +#endif PyThread_ident_t ident = PyThread_get_thread_ident_ex(); if (ident == PYTHREAD_INVALID_THREAD_ID) { PyErr_SetString(ThreadError, "no current thread ident"); From a9ca268922e15a91dfec76fcc7d69864d5cc9fca Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Fri, 21 Feb 2025 00:12:44 -0600 Subject: [PATCH 2/6] Add NEWS entry Signed-off-by: Vincent Fazio --- .../2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst new file mode 100644 index 00000000000000..1fe88d2c2b2912 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst @@ -0,0 +1,2 @@ +Fix an issue with the value of :func:`threading.get_ident` for the main thread +being sign-extended on some platforms. From 7a4de5f081d8201402004e2c43d46f4cd3084c0d Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Sat, 22 Feb 2025 19:43:26 -0600 Subject: [PATCH 3/6] Move pthread_t to ident conversion to a shared function Move this to a static function so all callers have consistent behavior. Signed-off-by: Vincent Fazio --- Python/thread_pthread.h | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index c010b3a827757f..f0a39533c5bd4d 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -306,6 +306,25 @@ do_start_joinable_thread(void (*func)(void *), void *arg, pthread_t* out_id) return 0; } +/* Helper to convert pthread_t to PyThread_ident_t. POSIX allows pthread_t to be + non-arithmetic, e.g., musl typedefs it as a pointer */ +static PyThread_ident_t +_pthread_t_to_ident(pthread_t value) { +#if SIZEOF_PTHREAD_T > SIZEOF_LONG + return (PyThread_ident_t) *(unsigned long *) &value; +#else + PyThread_ident_t ident; +#if defined(__linux__) && !defined(__GLIBC__) + ident = (PyThread_ident_t) (uintptr_t) value; + assert(pthread_equal(value, (pthread_t) (uintptr_t) ident)); +#else + ident = (PyThread_ident_t) value; + assert(pthread_equal(value, (pthread_t) ident)); +#endif + return ident; +#endif // SIZEOF_PTHREAD_T > SIZEOF_LONG +} + int PyThread_start_joinable_thread(void (*func)(void *), void *arg, PyThread_ident_t* ident, PyThread_handle_t* handle) { @@ -313,9 +332,8 @@ PyThread_start_joinable_thread(void (*func)(void *), void *arg, if (do_start_joinable_thread(func, arg, &th)) { return -1; } - *ident = (PyThread_ident_t) th; + *ident = _pthread_t_to_ident(th); *handle = (PyThread_handle_t) th; - assert(th == (pthread_t) *ident); assert(th == (pthread_t) *handle); return 0; } @@ -328,11 +346,7 @@ PyThread_start_new_thread(void (*func)(void *), void *arg) return PYTHREAD_INVALID_THREAD_ID; } pthread_detach(th); -#if SIZEOF_PTHREAD_T <= SIZEOF_LONG - return (unsigned long) th; -#else - return (unsigned long) *(unsigned long *) &th; -#endif + return (unsigned long) _pthread_t_to_ident(th);; } int @@ -357,8 +371,7 @@ PyThread_get_thread_ident_ex(void) { if (!initialized) PyThread_init_thread(); threadid = pthread_self(); - assert(threadid == (pthread_t) (PyThread_ident_t) threadid); - return (PyThread_ident_t) threadid; + return _pthread_t_to_ident(threadid); } unsigned long From af1233b283e1fd6343dad8a2a2605a8009bb4556 Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Sat, 22 Feb 2025 19:51:41 -0600 Subject: [PATCH 4/6] Reword NEWS entry Signed-off-by: Vincent Fazio --- .../2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst index 1fe88d2c2b2912..124da33f8836f6 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-21-00-12-24.gh-issue-130115.mF-rP6.rst @@ -1,2 +1 @@ -Fix an issue with the value of :func:`threading.get_ident` for the main thread -being sign-extended on some platforms. +Fix an issue with thread identifiers being sign-extended on some platforms. From 7bdfd34cdfc44eb8a8a3c1e799da43101af3f155 Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Sun, 23 Feb 2025 11:56:13 -0600 Subject: [PATCH 5/6] Revert "Return the main thread's short identifier on certain platforms" This reverts commit 51d657968f014cd0f661397658d37ba23221679f. --- Modules/_threadmodule.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 65f504253ba3ea..e251736fb36aa9 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -2029,13 +2029,6 @@ An obsolete synonym of allocate_lock()."); static PyObject * thread_get_ident(PyObject *self, PyObject *Py_UNUSED(ignored)) { - /* Work around an issue with the main thread ID to failing comparison checks - due to sign extension on some Linux libc implemenations. Can be removed - when thread identifiers are reworked. */ -#if SIZEOF_LONG < SIZEOF_LONG_LONG && defined(__linux__) && !defined(__GLIBC__) - if (_Py_IsMainThread()) - return PyLong_FromUnsignedLong(_PyRuntime.main_thread); -#endif PyThread_ident_t ident = PyThread_get_thread_ident_ex(); if (ident == PYTHREAD_INVALID_THREAD_ID) { PyErr_SetString(ThreadError, "no current thread ident"); From 140a4d8c98fd07f1cea65f8d743609ff6a9ae894 Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Wed, 2 Apr 2025 07:59:00 -0500 Subject: [PATCH 6/6] simplify conversion logic --- Python/thread_pthread.h | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index f0a39533c5bd4d..da4058242448f3 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -307,22 +307,21 @@ do_start_joinable_thread(void (*func)(void *), void *arg, pthread_t* out_id) } /* Helper to convert pthread_t to PyThread_ident_t. POSIX allows pthread_t to be - non-arithmetic, e.g., musl typedefs it as a pointer */ + non-arithmetic, e.g., musl typedefs it as a pointer. */ static PyThread_ident_t _pthread_t_to_ident(pthread_t value) { -#if SIZEOF_PTHREAD_T > SIZEOF_LONG - return (PyThread_ident_t) *(unsigned long *) &value; +// Cast through an integer type of the same size to avoid sign-extension. +#if SIZEOF_PTHREAD_T == SIZEOF_VOID_P + return (uintptr_t) value; +#elif SIZEOF_PTHREAD_T == SIZEOF_LONG + return (unsigned long) value; +#elif SIZEOF_PTHREAD_T == SIZEOF_INT + return (unsigned int) value; +#elif SIZEOF_PTHREAD_T == SIZEOF_LONG_LONG + return (unsigned long long) value; #else - PyThread_ident_t ident; -#if defined(__linux__) && !defined(__GLIBC__) - ident = (PyThread_ident_t) (uintptr_t) value; - assert(pthread_equal(value, (pthread_t) (uintptr_t) ident)); -#else - ident = (PyThread_ident_t) value; - assert(pthread_equal(value, (pthread_t) ident)); +#error "Unsupported SIZEOF_PTHREAD_T value" #endif - return ident; -#endif // SIZEOF_PTHREAD_T > SIZEOF_LONG } int