From a4afb55fb226e1debcdaaf80890b790198ba14cc Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Mon, 18 Jan 2021 18:14:10 +0300 Subject: [PATCH 1/2] bpo-42888: Remove PyThread_exit_thread() calls from top-level thread functions PyThread_exit_thread() uses pthread_exit() on POSIX systems. In glibc, pthread_exit() is implemented in terms of pthread_cancel(), requiring the stack unwinder implemented in libgcc. Further, in dynamically linked applications, calls of pthread_exit() in source code do not make libgcc_s.so a startup dependency: instead, it's lazily loaded by glibc via dlopen() when pthread_exit() is called the first time[1]. All of this makes otherwise finely working CPython fail in multithreaded applications on thread exit if dlopen() fails for any reason. While providing libgcc_s.so is the reponsibility of the user (or their package manager), this hidden dependency has been the source of countless frustrations(e.g. [2]) and, further, dlopen() may fail for other reasons([3]). But most calls to PyThread_exit_thread() in CPython are useless because they're done from the top-level thread function and hence are equivalent to simply returning. So remove all such calls, thereby avoiding the glibc cancellation machinery. The only exception are calls in take_gil() (Python/ceval_gil.h) which serve as a safety net for daemon threads attempting to acquire the GIL after Py_Finalize(). Unless a better model for daemon threads is devised or support for them is removed, those calls have to be preserved since we need to terminate the thread right now without touching any interpreter state. Of course, since PyThread_exit_thread() is a public API, any extension module can still call it and trip over the same issue. [1] https://sourceware.org/legacy-ml/libc-help/2014-07/msg00000.html [2] https://stackoverflow.com/questions/64797838/libgcc-s-so-1-must-be-installed-for-pthread-cancel-to-work [3] https://www.sourceware.org/bugzilla/show_bug.cgi?id=13119 --- Modules/_testcapimodule.c | 2 -- Modules/_threadmodule.c | 2 -- Programs/_testembed.c | 2 -- Python/thread_nt.h | 10 +++++++++- Python/thread_pthread.h | 9 ++++++++- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 4f97927fa23229..0b8d8b3cdc07c0 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4512,8 +4512,6 @@ temporary_c_thread(void *data) PyGILState_Release(state); PyThread_release_lock(test_c_thread->exit_event); - - PyThread_exit_thread(); } static PyObject * diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 9b8757715a0b9b..2e1fff3ad92e95 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1087,8 +1087,6 @@ thread_run(void *boot_raw) tstate->interp->num_threads--; PyThreadState_Clear(tstate); _PyThreadState_DeleteCurrent(tstate); - - PyThread_exit_thread(); } static PyObject * diff --git a/Programs/_testembed.c b/Programs/_testembed.c index 52c56746813a34..87a1da30982341 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -241,8 +241,6 @@ static void bpo20891_thread(void *lockp) PyGILState_Release(state); PyThread_release_lock(lock); - - PyThread_exit_thread(); } static int test_bpo20891(void) diff --git a/Python/thread_nt.h b/Python/thread_nt.h index 05b982d32dc526..6c7d93d75bf1fc 100644 --- a/Python/thread_nt.h +++ b/Python/thread_nt.h @@ -160,6 +160,13 @@ PyThread__init_thread(void) * Thread support. */ +static void +_pythread_at_thread_exit(void) +{ + dprintf(("%lu: _pythread_at_thread_exit called\n", + PyThread_get_thread_ident())); +} + typedef struct { void (*func)(void*); void *arg; @@ -175,6 +182,7 @@ bootstrap(void *call) void *arg = obj->arg; HeapFree(GetProcessHeap(), 0, obj); func(arg); + _pythread_at_thread_exit(); return 0; } @@ -254,7 +262,7 @@ PyThread_get_thread_native_id(void) void _Py_NO_RETURN PyThread_exit_thread(void) { - dprintf(("%lu: PyThread_exit_thread called\n", PyThread_get_thread_ident())); + _pythread_at_thread_exit(); if (!initialized) exit(0); _endthreadex(0); diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index ec7d737518b68c..60bbe2b232d03b 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -218,6 +218,12 @@ PyThread__init_thread(void) * Thread support. */ +static void +_pythread_at_thread_exit(void) +{ + dprintf(("_pythread_at_thread_exit called\n")); +} + /* bpo-33015: pythread_callback struct and pythread_wrapper() cast "void func(void *)" to "void* func(void *)": always return NULL. @@ -238,6 +244,7 @@ pythread_wrapper(void *arg) PyMem_RawFree(arg); func(func_arg); + _pythread_at_thread_exit(); return NULL; } @@ -359,7 +366,7 @@ PyThread_get_thread_native_id(void) void _Py_NO_RETURN PyThread_exit_thread(void) { - dprintf(("PyThread_exit_thread called\n")); + _pythread_at_thread_exit(); if (!initialized) exit(0); pthread_exit(0); From c256c87d447591cca2ff9f03538f88643bd208d9 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Thu, 21 Jan 2021 21:17:31 +0300 Subject: [PATCH 2/2] Add NEWS entry --- .../next/Library/2021-01-21-18-03-09.bpo-42888.0oAR2x.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-01-21-18-03-09.bpo-42888.0oAR2x.rst diff --git a/Misc/NEWS.d/next/Library/2021-01-21-18-03-09.bpo-42888.0oAR2x.rst b/Misc/NEWS.d/next/Library/2021-01-21-18-03-09.bpo-42888.0oAR2x.rst new file mode 100644 index 00000000000000..d77cafcb961ec0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-01-21-18-03-09.bpo-42888.0oAR2x.rst @@ -0,0 +1,4 @@ +Remove calls of :c:func:`PyThread_exit_thread()` from top-level thread +functions, thereby avoiding a runtime dependency on ``libgcc_s.so`` and +associated issues with lazy-loading it via ``dlopen()`` in typical scenarios +on glibc-based Linux systems.