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

_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 opened this issue Jun 16, 2021 · 18 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

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
    Copy link
    Member Author

    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 vstinner added 3.11 only security fixes stdlib Python modules in the Lib dir labels Jun 16, 2021
    @vstinner
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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:

    @vstinner
    Copy link
    Member Author

    _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
    Copy link
    Member Author

    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
    Copy link
    Member Author

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

    @vstinner
    Copy link
    Member Author

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

    @vstinner
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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
    Copy link
    Member Author

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

    @vstinner vstinner added 3.9 only security fixes 3.10 only security fixes labels Jun 21, 2021
    @vstinner vstinner added 3.9 only security fixes 3.10 only security fixes labels Jun 21, 2021
    @vstinner
    Copy link
    Member Author

    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
    Copy link
    Member Author

    Se also bpo-35866 which looks like a duplicate.

    @vstinner
    Copy link
    Member Author

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

    @vstinner
    Copy link
    Member Author

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

    @vstinner
    Copy link
    Member Author

    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
    Copy link
    Member Author

    vstinner commented Jul 3, 2021

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

    @vstinner
    Copy link
    Member Author

    vstinner commented Jul 8, 2021

    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
    Copy link
    Member Author

    vstinner commented Jul 8, 2021

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant