python / cpython

The Python programming language
https://www.python.org
Other
63.28k stars 30.3k forks source link

_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 #88600

Closed vstinner closed 3 years ago

vstinner commented 3 years ago
BPO 44434
Nosy @vstinner, @miss-islington, @erlend-aasland
PRs
  • python/cpython#26758
  • python/cpython#26824
  • python/cpython#26825
  • python/cpython#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: ```python assignee = None closed_at = created_at = 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 = user = 'https://github.com/vstinner' ``` bugs.python.org fields: ```python activity = actor = 'vstinner' assignee = 'none' closed = True closed_date = closer = 'vstinner' components = ['Library (Lib)'] creation = 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 commented 3 years ago

    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 commented 3 years ago

    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 commented 3 years ago

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

    commit 0d5e52d3469a310001afe50689f77ddba6d554d1 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:

    vstinner commented 3 years ago

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

    commit 1984f1e1c6306d4e8073c28d2395638f80ea509b 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 bcc207484a0f8f27a684e11194e7430c0710f66d 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 commented 3 years ago

    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:

    (...)

    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 commented 3 years ago

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

    vstinner commented 3 years ago

    New changeset 45a78f906d2d5fe5381d78466b11763fc56d57ba by Victor Stinner in branch 'main': bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758) https://github.com/python/cpython/commit/45a78f906d2d5fe5381d78466b11763fc56d57ba

    vstinner commented 3 years ago

    New changeset 83ad40efc3e299d1e94692d958111a63c2fd6775 by Victor Stinner in branch '3.9': bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758) (GH-26825) https://github.com/python/cpython/commit/83ad40efc3e299d1e94692d958111a63c2fd6775

    vstinner commented 3 years ago

    New changeset 6614eac843c5dc0f4c2664ef610b81e556e44923 by Miss Islington (bot) in branch '3.10': bpo-44434: Don't call PyThread_exit_thread() explicitly (GH-26758) (GH-26824) https://github.com/python/cpython/commit/6614eac843c5dc0f4c2664ef610b81e556e44923

    vstinner commented 3 years ago

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

    vstinner commented 3 years ago

    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 commented 3 years ago

    Se also bpo-35866 which looks like a duplicate.

    vstinner commented 3 years ago

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

    vstinner commented 3 years ago

    New changeset 48e3a1d95aee013974121fcafe19816c0e9a41da by Victor Stinner in branch 'main': bpo-44434: Remove useless calls to PyThread_exit_thread() (GH-26943) https://github.com/python/cpython/commit/48e3a1d95aee013974121fcafe19816c0e9a41da

    vstinner commented 3 years ago

    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 commented 3 years ago

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

    vstinner commented 3 years ago

    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 commented 3 years ago

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