Skip to content

PyOS_Readline drops GIL and calls PyOS_StdioReadline, which isn't thread safe #60946

Closed
Closed
PyOS_Readline drops GIL and calls PyOS_StdioReadline, which isn't thread safe#60946
Assignees
tpn
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@tpn
tpn
opened on Dec 21, 2012
BPO 16742
Nosy @gpshead, @pitrou, @kristjanvalur, @vstinner, @tpn, @meadori
Dependencies
  • bpo-3329: API for setting the memory allocator used by Python
    		•
    Files
  • readline_gil.patch
  • readline_gil-2.patch
    		•

    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 = 'https://github.com/tpn'
closed_at = <Date 2013-10-10.14:19:38.937>
created_at = <Date 2012-12-21.10:39:50.350>
labels = ['interpreter-core', 'type-bug']
title = "PyOS_Readline drops GIL and calls PyOS_StdioReadline, which isn't thread safe"
updated_at = <Date 2013-10-19.11:34:40.896>
user = 'https://github.com/tpn'

    bugs.python.org fields:

    activity = <Date 2013-10-19.11:34:40.896>
actor = 'vstinner'
assignee = 'trent'
closed = True
closed_date = <Date 2013-10-10.14:19:38.937>
closer = 'python-dev'
components = ['Interpreter Core']
creation = <Date 2012-12-21.10:39:50.350>
creator = 'trent'
dependencies = ['3329']
files = ['30578', '30593']
hgrepos = []
issue_num = 16742
keywords = ['patch']
message_count = 10.0
messages = ['177874', '188397', '188499', '191092', '191094', '191178', '199388', '200340', '200390', '200404']
nosy_count = 7.0
nosy_names = ['gregory.p.smith', 'pitrou', 'kristjan.jonsson', 'vstinner', 'trent', 'meador.inge', 'python-dev']
pr_nums = []
priority = 'critical'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue16742'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

    Activity

    1. tpn commented on Dec 21, 2012

      @tpn
      tpn
      on Dec 21, 2012
      MemberAuthor

      Relevant thread: http://mail.python.org/pipermail/python-dev/2012-December/123225.html

      PyOS_StdioReadline features numerous calls that require the GIL to be held. Ideally, the GIL drop-take should be moved closer to the actual underlying read system call.

    2. tpn
      self-assigned this
      on Dec 21, 2012
    3. tpn
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      type-bugAn unexpected behavior, bug, or error
      on Dec 21, 2012

    4. pitrou commented on May 4, 2013

      @pitrou
      pitrou
      on May 4, 2013
      Member

      So, could you propose a patch?

    5. kristjanvalur commented on May 6, 2013

      @kristjanvalur
      kristjanvalurmannequin
      on May 6, 2013
      Mannequin

      My quick and dirty fix is simple:

          _PyOS_ReadlineTState = PyThreadState_GET();
    /* CCP change, cannot release the GIL here because PyOS_StdioReadline uses
     * the regular MALLOC
     */
    /*
    Py_BEGIN_ALLOW_THREADS
    */
#ifdef WITH_THREAD
    PyThread_acquire_lock(_PyOS_ReadlineLock, 1);
#endif
      /* This is needed to handle the unlikely case that the
 * interpreter is in interactive mode *and* stdin/out are not
 * a tty.  This can happen, for example if python is run like
 * this: python -i < test1.py
 */
if (!isatty (fileno (sys_stdin)) || !isatty (fileno (sys_stdout)))
    rv = PyOS_StdioReadline (sys_stdin, sys_stdout, prompt);
else
    rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout,
                                         prompt);
/*
Py_END_ALLOW_THREADS
*/

      #ifdef WITH_THREAD
    PyThread_release_lock(_PyOS_ReadlineLock);
#endif

      Basically, we just comment out the lock release since we don't need it. The reason we found this was that we were using GIL a custom mallocator which should have been run with the GIL but wasn´t.

    6. vstinner commented on Jun 13, 2013

      @vstinner
      vstinner
      on Jun 13, 2013
      Member

      I just found the readline/GIL issue while working on bpo-18203. I created bpo-18205 but then I found this issue. I just closed bpo-18205 as a duplicate. Here is a patch for Python 3.4.

      --

      Copy of the initial message (msg191089):

      The callback PyOS_ReadlineFunctionPointer (used to read a line from the standard input) must return a buffer allocated by PyMem_Malloc(), but PyOS_Readline() releases the GIL before calling PyOS_ReadlineFunctionPointer.

      Simplified extract of PyOS_Readline():

      Py_BEGIN_ALLOW_THREADS
if (!isatty (fileno (sys_stdin)) || !isatty (fileno (sys_stdout)))
    rv = PyOS_StdioReadline (sys_stdin, sys_stdout, prompt);
else
    rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout,
                                         prompt);
Py_END_ALLOW_THREADS

      tok_nextc() calls PyOS_Readline() and calls PyMem_FREE() to release its result.

      PyOS_ReadlineFunctionPointer should allocate memory using malloc(), not using PyMem_Malloc(). But PyOS_Readline() should copy the line into a buffer allocated by PyMem_Malloc() to keep backward compatibility.

      See also issue bpo-18203 and bpo-3329.

    7. vstinner commented on Jun 13, 2013

      @vstinner
      vstinner
      on Jun 13, 2013
      Member

      See the following thread on python-dev, the root problem is that PyMem_Malloc() cannot be called with the GIL held. This is a bug in my opinion, and it should be fixed.
      http://mail.python.org/pipermail/python-dev/2013-June/126822.html

    8. vstinner commented on Jun 15, 2013

      @vstinner
      vstinner
      on Jun 15, 2013
      Member

      Updated patch for the final API of bpo-3329. Update also the documentation. PyOS_ReadlineFunctionPointer must now use PyMem_RawMalloc() or PyMem_RawRealloc(), instead of PyMem_Malloc() or PyMem_Realloc().

    9. python-dev commented on Oct 10, 2013

      @python-dev
      python-devmannequin
      on Oct 10, 2013
      Mannequin

      New changeset 98dbe677dfe7 by Victor Stinner in branch 'default':
      Close bpo-16742: Fix misuse of memory allocations in PyOS_Readline()
      http://hg.python.org/cpython/rev/98dbe677dfe7

    10. python-dev
      closed this as completedon Oct 10, 2013

    11. python-dev commented on Oct 19, 2013

      @python-dev
      python-devmannequin
      on Oct 19, 2013
      Mannequin

      New changeset 6c9050ad1afc by Victor Stinner in branch 'default':
      Issue bpo-16742: My fix on PyOS_StdioReadline() was incomplete, PyMem_FREE() was
      http://hg.python.org/cpython/rev/6c9050ad1afc

    12. kristjanvalur commented on Oct 19, 2013

      @kristjanvalur
      kristjanvalurmannequin
      on Oct 19, 2013
      Mannequin

      Perhaps in debug builds the memory apis should verify consistency and matching useage.

    13. vstinner commented on Oct 19, 2013

      @vstinner
      vstinner
      on Oct 19, 2013
      Member

      Kristján Valur Jónsson added the comment:

      Perhaps in debug builds the memory apis should verify consistency and
      matching useage.

      Python does check usage of apis in debug mode. Memory allocation failure
      are almost never checked. See my pyfailmalloc module for that.

    14. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions