_thread module: Remove redundant PyThread_exit_thread() call to avoid glibc fatal error: libgcc_s.so.1 must be installed for pthread_cancel to work

[vstinner]

BPO	44434
Nosy	@vstinner, @miss-islington, @erlend-aasland
PRs	bpo-44434: Don't call PyThread_exit_thread() explicitly #26758 [3.10] bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758) #26824 [3.9] bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758) #26825 bpo-44434: Remove useless calls to PyThread_exit_thread() #26943
Files	pthread_cancel_bug.py pthread_cancel_emfile.py

^{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:

assignee = None
closed_at = <Date 2021-06-21.12:29:46.712>
created_at = <Date 2021-06-16.14:19:11.182>
labels = ['library', '3.9', '3.10', '3.11']
title = '_thread module: Remove redundant PyThread_exit_thread() call to avoid glibc fatal error: libgcc_s.so.1 must be installed for pthread_cancel to work'
updated_at = <Date 2021-07-08.09:25:04.498>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2021-07-08.09:25:04.498>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-21.12:29:46.712>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2021-06-16.14:19:11.182>
creator = 'vstinner'
dependencies = []
files = ['50112', '50113']
hgrepos = []
issue_num = 44434
keywords = ['patch']
message_count = 18.0
messages = ['395924', '395925', '395930', '395931', '395937', '395941', '396229', '396236', '396239', '396240', '396672', '396676', '396678', '396682', '396683', '396928', '397129', '397132']
nosy_count = 3.0
nosy_names = ['vstinner', 'miss-islington', 'erlendaasland']
pr_nums = ['26758', '26824', '26825', '26943']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue44434'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[vstinner]

The glibc pthread_exit() functions loads an unwind function from libgcc_s.so.1 using dlopen(). dlopen() can fail to open libgcc_s.so.1 file to various reasons, but the most likely seems to be that the process is out of available file descriptor (EMFILE error).

If the glibc pthread_exit() fails to open libgcc_s.so.1, it aborts the process. Extract of pthread_cancel():

  /* Trigger an error if libgcc_s cannot be loaded.  */
  {
    struct unwind_link *unwind_link = __libc_unwind_link_get ();
    if (unwind_link == NULL)
      __libc_fatal (LIBGCC_S_SO
		    " must be installed for pthread_cancel to work\n");
  }

Sometimes, libgcc_s.so.1 library is loaded early in Python startup. Sometimes, it only loaded when the first Python thread exits.

Hitting in a multithreaded real world application, dlopen() failing with EMFILE is not deterministic. It depends on precise timing and in which order threads are running. It is unlikely in a small application, but it is more likely on a network server which has thousands of open sockets (file descriptors).

--

Attached scripts reproduces the issue. You may need to run the scripts (especially pthread_cancel_emfile.py) multiple times to trigger the issue. Sometimes libgcc_s library is loaded early for an unknown reason, it works around the issue.

(1) pthread_cancel_bug.py

$ python3.10 pthread_cancel_bug.py 
libgcc_s.so.1 must be installed for pthread_cancel to work
Abandon (core dumped)

(2) pthread_cancel_emfile.py:

$ python3.10 ~/pthread_cancel_emfile.py 
spawn thread
os.open failed: OSError(24, 'Too many open files')
FDs open by the thread: 2 (max FD: 4)
fd 0 valid? True
fd 1 valid? True
fd 2 valid? True
fd 3 valid? True
fd 4 valid? True
libgcc_s.so.1 must be installed for pthread_cancel to work
Abandon (core dumped)

--

Example of real world issue on RHEL8:
https://bugzilla.redhat.com/show_bug.cgi?id=1972293

The RHEL reproducer uses a very low RLIMIT_NOFILE (5 file descriptors) to trigger the bug faster. It simulates a busy server application.

--

There are different options:

() Modify thread_run() of Modules/_threadmodule.c to remove the *redundant PyThread_exit_thread() call.

This is the most simple option and it sounds perfectly safe to me. I'm not sure why PyThread_exit_thread() is called explicitly. We don't pass any parameter to the function.

(*) Link the Python _thread extension on libgcc_s.so if Python it built with the glibc.

Checking if Python is linked to the glibc is non trivial and we have hardcode the "libgcc_s" library name. I expect painful maintenance burden with this option.

(*) Load explicitly the libgcc_s.so library in _thread.start_new_thread(): when the first thread is created.

We need to detect that we are running the glibc at runtime, by calling confstr('CS_GNU_LIBC_VERSION') for example. The problem is that "libgcc_s.so.1" filename may change depending on the Linux distribution. It will likely have a different filename on macOS (".dynlib"). In short, it's tricky to get it right.

(*) Fix the glibc!

I discussed with glibc developers who explained me that there are good reasons to keep the unwind code in the compiler (GCC), and so load it dynamically in the glibc. In short, this is not going to change.

--

Attached PR implements the most straightforward option: remove the redundant PyThread_exit_thread() call in thread_run().

[vstinner]

See also bpo-18748 "io.IOBase destructor silence I/O error on close() by default" which was caused by a bug in an application, the application closed the libgcc_s file descriptor by mistake. It closed the same file decriptor twice, whereas the FD was reused by dlopen() in the meanwhile. But the result was the same, the process aborted with this error message:

"libgcc_s.so.1 must be installed for pthread_cancel to work"

[vstinner]

PyThread_exit_thread() was modified in 2011 to fix daemon threads:

commit 0d5e52d
Author: Antoine Pitrou <solipsis@pitrou.net>
Date: Wed May 4 20:02:30 2011 +0200

Issue bpo-1856: Avoid crashes and lockups when daemon threads run while the
interpreter is shutting down; instead, these threads are now killed when
they try to take the GIL.

 PyThread_exit_thread(void)
 {
     dprintf(("PyThread_exit_thread called\n"));
-    if (!initialized) {
+    if (!initialized)
         exit(0);
-    }
+    pthread_exit(0);
 }

This change remains important for Python/ceval.c. When a daemon thread tries to acquire the GIL, it calls PyThread_exit_thread() if Python already exited to exit immediately the thread. Example from take_gil():

    if (tstate_must_exit(tstate)) {
        /* bpo-39877: If Py_Finalize() has been called and tstate is not the
           thread which called Py_Finalize(), exit immediately the thread.

           This code path can be reached by a daemon thread after Py_Finalize()
           completes. In this case, tstate is a dangling pointer: points to
           PyThreadState freed memory. */
        PyThread_exit_thread();
    }

See also my articles on daemon threads fixes:

• https://vstinner.github.io/gil-bugfixes-daemon-threads-python39.html
• https://vstinner.github.io/daemon-threads-python-finalization-python32.html

[vstinner]

_thread.start_new_thread() always called "exit thread", since the function was added to Python:

commit 1984f1e
Author: Guido van Rossum <guido@python.org>
Date: Tue Aug 4 12:41:02 1992 +0000

* Makefile adapted to changes below.
* split pythonmain.c in two: most stuff goes to pythonrun.c, in the library.
* new optional built-in threadmodule.c, build upon Sjoerd's thread.{c,h}.
* new module from Sjoerd: mmmodule.c (dynamically loaded).
* new module from Sjoerd: sv (svgen.py, svmodule.c.proto).
* new files thread.{c,h} (from Sjoerd).
* new xxmodule.c (example only).
* myselect.h: bzero -> memset
* select.c: bzero -> memset; removed global variable

static void
t_bootstrap(args_raw)
        void *args_raw;
{
        object *args = (object *) args_raw;
        object *func, *arg, *res;

        restore_thread((void *)NULL);
        func = gettupleitem(args, 0);
        arg = gettupleitem(args, 1);
        res = call_object(func, arg);
        DECREF(arg); /* Matches the INCREF(arg) in thread_start_new_thread */
        if (res == NULL) {
                fprintf(stderr, "Unhandled exception in thread:\n");
                print_error(); /* From pythonmain.c */
                fprintf(stderr, "Exiting the entire program\n");
                goaway(1);
        }
        (void) save_thread();
        exit_thread();
}

exit_thread() was partially replaced with PyThread_exit_thread() in:

commit bcc2074
Author: Guido van Rossum <guido@python.org>
Date: Tue Aug 4 22:53:56 1998 +0000

Changes for BeOS, QNX and long long, by Chris Herborth.

[vstinner]

Unix pthread_create() manual page.
https://man7.org/linux/man-pages/man3/pthread_create.3.html

The new thread terminates in one of the following ways:

(...)

• It returns from start_routine(). This is equivalent to calling pthread_exit(3) with the value supplied in
  the return statement.
  (...)

Calling pthread_exit(0) is optional.

--

MSDN _beginthreadex() documentation:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/beginthread-beginthreadex?view=msvc-160

"When the thread returns from that routine, it is terminated automatically."

Calling _endthreadex(0) is optional.

[vstinner]

See also bpo-44436 "[Windows] _thread.start_new_thread() should close the thread handle".

[vstinner]

New changeset 45a78f9 by Victor Stinner in branch 'main':
bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758)
45a78f9

[vstinner]

New changeset 83ad40e by Victor Stinner in branch '3.9':
bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758) (GH-26825)
83ad40e

[vstinner]

New changeset 6614eac by Miss Islington (bot) in branch '3.10':
bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758) (GH-26824)
6614eac

[vstinner]

Ok, the issue is now fixed in 3.9, 3.10 and main branches.

[vstinner]

I marked bpo-42888 as a duplicate of this issue.

I created PR 26943 based on Alexey's PR 24241 to complete my fix (remove two calls in two tests).

Copy of his interesting PR commit message:
---
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
---

[vstinner]

Se also bpo-35866 which looks like a duplicate.

[vstinner]

I marked bpo-37395 "Core interpreter should be linked with libgcc_s.so on Linux" as a duplicate of this issue.

[vstinner]

New changeset 48e3a1d by Victor Stinner in branch 'main':
bpo-44434: Remove useless calls to PyThread_exit_thread() (GH-26943)
48e3a1d

[vstinner]

On Linux, there is a workaround for Python versions which don't include this fix:

$ LD_PRELOAD=/usr/lib64/libgcc_s.so.1 python3 ...

To preload the libgcc_s.so.1 library in the Python process when running Python.

[vstinner]

Good news: this change fixed bpo-35866 "concurrent.futures deadlock".

[vstinner]

I started "Does anyone use threading debug PYTHONTHREADDEBUG=1 env var? Can I remove it?" thread on python-dev:
https://mail.python.org/archives/list/python-dev@python.org/thread/NMLGCDRUKLZSTK4UICJTKR54WRXU2ZGJ/

[vstinner]

I created bpo-44584: "Deprecate thread debugging PYTHONTHREADDEBUG=1".