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

Call _PyGILState_Init() earlier in Py_InitializeEx() #52310

Closed
vstinner opened this issue Mar 4, 2010 · 11 comments
Closed

Call _PyGILState_Init() earlier in Py_InitializeEx() #52310

vstinner opened this issue Mar 4, 2010 · 11 comments
Labels
type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@vstinner
Copy link
Member

vstinner commented Mar 4, 2010

BPO 8063
Nosy @tim-one, @loewis, @amauryfa, @pitrou, @vstinner
Files
  • gil_state_init-py3k.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 = None
    closed_at = <Date 2010-08-17.22:28:14.790>
    created_at = <Date 2010-03-04.23:09:55.689>
    labels = ['type-crash']
    title = 'Call _PyGILState_Init() earlier in Py_InitializeEx()'
    updated_at = <Date 2010-08-17.22:28:14.788>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2010-08-17.22:28:14.788>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-08-17.22:28:14.790>
    closer = 'vstinner'
    components = []
    creation = <Date 2010-03-04.23:09:55.689>
    creator = 'vstinner'
    dependencies = []
    files = ['18559']
    hgrepos = []
    issue_num = 8063
    keywords = ['patch']
    message_count = 11.0
    messages = ['100432', '100433', '100443', '100618', '100620', '100622', '100623', '100624', '100668', '114183', '114185']
    nosy_count = 5.0
    nosy_names = ['tim.peters', 'loewis', 'amaury.forgeotdarc', 'pitrou', 'vstinner']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue8063'
    versions = ['Python 2.7', 'Python 3.2']

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 4, 2010

    _PyGILState_Init() initialize autoInterpreterState variable. This variable have to be set before the first call to PyGILState_Ensure().

    The problem is that _PyGILState_Init() is called late: at the end of Py_InitializeEx(). It's called after initsite(), whereas initsite() may need to call _PyGILState_Init().

    Example: add the following lines at the end of site.py:
    ---

    import readline
    import rlcompleter
    readline.parse_and_bind("tab: complete")
    raw_input("press TAB")

    Run an interpreter and press TAB:
    ---

    $ ./python
    press TABpython: Python/pystate.c:595: PyGILState_Ensure: Assertion `autoInterpreterState' failed.
    Abandon

    Other example of functiions using _PyGILState_Init(): _PyObject_Dump() (useful for debug), sqlite module, os.popen*(), ctypes Python callbacks, etc.

    I don't know the right place for _PyGILState_Init() in Py_InitializeEx(). _PyGILState_Init() calls the following functions:

    • PyThread_get_thread_ident()
    • PyThread_allocate_lock()
    • PyThread_acquire_lock()
    • PyThread_release_lock()
    • Py_FatalError() (on error)

    None of these function use Python functions, so _PyGILState_Init() can be called very early. The earliest position is just after the call to PyThreadState_New(), before PyThreadState_Swap(). It tested it and it works correctly.

    If _PyGILState_Init() is called before PyThreadState_Swap(), PyThreadState_Swap() can be simplified (it doesn't need to check if GIL is initialized or not). Attached patch implement that.

    --

    I hit this bug when I was debuging Python: I added a call to _PyObject_Dump() which stopped Python (assertion error) because the GIL was not initialized :-/

    I already hitted this bug some weeks ago when I was working on the thread state preallocation when creating a new thread: bpo-7544.

    @vstinner vstinner added the type-crash A hard crash of the interpreter, possibly with a core dump label Mar 4, 2010
    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 4, 2010

    haypo> I already hitted this bug some weeks ago when I was working
    haypo> on the thread state preallocation when creating a new thread:
    haypo> bpo-7544.

    According to msg97715: it was indirectly the same issue, call _PyObject_Dump() while the GIL is not initialized.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 5, 2010

    Bootstrap issues are always quite delicate, and I'd suggest being careful in this case.

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 8, 2010

    See also bpo-3137.

    @amauryfa
    Copy link
    Member

    amauryfa commented Mar 8, 2010

    I would keep the call to PyThreadState_Swap() next to PyThreadState_New(): create the thread state and install it. _PyGILState_Init() may come after.

    @amauryfa
    Copy link
    Member

    amauryfa commented Mar 8, 2010

    haypo, it seems you removed the initial message...

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 8, 2010

    haypo, it seems you removed the initial message...

    I know... My mouse clicked on [remove] button, it wasn't me! I don't know how to restore it. The message: msg100432.

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 8, 2010

    I found the following issue in Roundup tracker to restore a deleted message:
    http://psf.upfronthosting.co.za/roundup/meta/issue267

    I tried to write the URL, but it doesn't work:
    http://bugs.python.org/issue8063?@action=edit&@add@messages=100432

    I tried also using Poster Firefox extension (to post a POST request, not a GET request), but it doesn't work. My URL should be wrong :-/

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 8, 2010

    My SIGINT.patch for bpo-3137 moves the call to _PyGILState_Init() just before initsite(), so it doesn't change too much Py_InitializeEx() and it's enough for me :-)

    @vstinner
    Copy link
    Member Author

    While working on bpo-9425, I usually hit two annoying issues:

    • _PyObject_Dump() crashs (assertion error) if I call it (with gdb) in Py_InitializeEx()
    • because of python-gdb.py, gdb does segfault (I don't know yet where it does come from)

    So I'm back on the GIL topic: I still would like to initialize the GIL earlier in Py_InitializeEx(). As Amaury wrote, I think that the right place is just after "(void) PyThreadState_Swap(tstate);". This is exactly what does my new patch (for py3k).

    I think that only python 3.2 should be patched.

    @vstinner vstinner reopened this Aug 17, 2010
    @vstinner
    Copy link
    Member Author

    Commited as r84163 to 3.2. Don't backport because it is not really a bug and I prefer to avoid touching stable branches with such minor details.

    @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
    type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants