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

repeated Py_Initialize/PyRun_SimpleString/Py_Finalize segfaults #71923

Closed
smcv mannequin opened this issue Aug 11, 2016 · 13 comments
Closed

repeated Py_Initialize/PyRun_SimpleString/Py_Finalize segfaults #71923

smcv mannequin opened this issue Aug 11, 2016 · 13 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@smcv
Copy link
Mannequin

smcv mannequin commented Aug 11, 2016

BPO 27736
Nosy @brettcannon, @ned-deily, @zhangyangyu, @JelleZijlstra, @smcv
Files
  • issue27736.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 2016-08-16.04:24:58.635>
    created_at = <Date 2016-08-11.08:46:27.894>
    labels = ['interpreter-core', 'type-crash']
    title = 'repeated Py_Initialize/PyRun_SimpleString/Py_Finalize segfaults'
    updated_at = <Date 2016-08-16.04:24:58.634>
    user = 'https://github.com/smcv'

    bugs.python.org fields:

    activity = <Date 2016-08-16.04:24:58.634>
    actor = 'ned.deily'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-08-16.04:24:58.635>
    closer = 'ned.deily'
    components = ['Interpreter Core']
    creation = <Date 2016-08-11.08:46:27.894>
    creator = 'smcv'
    dependencies = []
    files = ['44093']
    hgrepos = []
    issue_num = 27736
    keywords = ['patch']
    message_count = 13.0
    messages = ['272423', '272424', '272560', '272564', '272586', '272587', '272612', '272776', '272791', '272794', '272820', '272824', '272825']
    nosy_count = 6.0
    nosy_names = ['brett.cannon', 'ned.deily', 'python-dev', 'xiang.zhang', 'JelleZijlstra', 'smcv']
    pr_nums = []
    priority = None
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue27736'
    versions = ['Python 3.6']

    @smcv
    Copy link
    Mannequin Author

    smcv mannequin commented Aug 11, 2016

    dbus-python has a regression test for https://bugs.freedesktop.org/show_bug.cgi?id=23831 which repeatedly initializes the interpreter, imports dbus and finalizes the interpreter. This test passes in Python up to 3.5, but is failing under Python 3.6 nightly builds on Travis-CI, and in Python 3.6.0a3 (package version 3.6.0~a3-1) on Debian.

    I've been able to reproduce the crash without anything specific to dbus with this C code:

    #include <stdio.h>
    
    #include <Python.h>
    
    int main(void)
    {
        int i;
    puts("1..1");
    
        for (i = 0; i < 100; ++i) {
            Py_Initialize();
            if (PyRun_SimpleString("\n") != 0) {
                puts("not ok 1 - there was an exception");
                return 1;
            }
            Py_Finalize();
        }
    puts("ok 1 - was able to loop 100 times");
    
        return 0;
    }

    It appears the crash is reliably in the 10th repeat:

    Program received signal SIGSEGV, Segmentation fault.
    0x00007ffff783a4bb in type_dealloc (type=0x7ffff7d8ad80 <DirEntryType>)
    at ../Objects/typeobject.c:3032
    3032 ../Objects/typeobject.c: No such file or directory.
    (gdb) bt
    #0 0x00007ffff783a4bb in type_dealloc (type=0x7ffff7d8ad80 <DirEntryType>)
    at ../Objects/typeobject.c:3032
    #1 0x00007ffff7817a1b in insertdict (value=<optimized out>, hash=<optimized out>,
    key=<optimized out>, mp=<optimized out>) at ../Objects/dictobject.c:806
    #2 PyDict_SetItem (
    op=op@entry={'open': None, 'O_DIRECT': None, 'chdir': None, 'O_ACCMODE': None, '__package__': None, 'WCOREDUMP': None, 'setgroups': None, 'O_CREAT': None, 'O_CLOEXEC': None, 'chown': None, 'sched_getscheduler': None, 'RTLD_NODELETE': None, 'terminal_size': None, 'EX_IOERR': None, 'sched_setaffinity': None, 'XATTR_SIZE_MAX': None, 'fstat': None, 'sched_rr_get_interval': None, 'O_LARGEFILE': None, 'times_result': None, 'get_inheritable': None, 'WIFEXITED': None, 'ST_NODEV': None, 'forkpty': None, 'ctermid': None, 'O_RSYNC': None, 'SCHED_FIFO': None, 'stat': None, 'replace': None, 'EX_NOINPUT': None, 'WUNTRACED': None, 'set_blocking': None, '_have_functions': None, 'unsetenv': None, 'setresgid': None, 'fchown': None, 'getgrouplist': None, 'openpty': None, 'lockf': None, 'chroot': None, 'readv': None, 'EX_NOHOST': None, 'error': None, 'WEXITSTATUS': None, 'WIFSIGNALED': None, 'WNOHANG': None, 'POSIX_FADV_WILLNEED': None, 'SEEK_HOLE': None, 'dup': None, 'POSIX_FADV_NOREUSE': None, 'kill': None, 'statvfs_result': None, 'WIFCON...(truncated),
    key='DirEntry', value=value@entry=None) at ../Objects/dictobject.c:1228
    #3 0x00007ffff782659c in _PyModule_ClearDict (
    d={'open': None, 'O_DIRECT': None, 'chdir': None, 'O_ACCMODE': None, '__package__': None, 'WCOREDUMP': None, 'setgroups': None, 'O_CREAT': None, 'O_CLOEXEC': None, 'chown': None, 'sched_getscheduler': None, 'RTLD_NODELETE': None, 'terminal_size': None, 'EX_IOERR': None, 'sched_setaffinity': None, 'XATTR_SIZE_MAX': None, 'fstat': None, 'sched_rr_get_interval': None, 'O_LARGEFILE': None, 'times_result': None, 'get_inheritable': None, 'WIFEXITED': None, 'ST_NODEV': None, 'forkpty': None, 'ctermid': None, 'O_RSYNC': None, 'SCHED_FIFO': None, 'stat': None, 'replace': None, 'EX_NOINPUT': None, 'WUNTRACED': None, 'set_blocking': None, '_have_functions': None, 'unsetenv': None, 'setresgid': None, 'fchown': None, 'getgrouplist': None, 'openpty': None, 'lockf': None, 'chroot': None, 'readv': None, 'EX_NOHOST': None, 'error': None, 'WEXITSTATUS': None, 'WIFSIGNALED': None, 'WNOHANG': None, 'POSIX_FADV_WILLNEED': None, 'SEEK_HOLE': None, 'dup': None, 'POSIX_FADV_NOREUSE': None, 'kill': None, 'statvfs_result': None, 'WIFCON...(truncated))
    at ../Objects/moduleobject.c:593
    #4 0x00007ffff782672e in _PyModule_Clear (m=m@entry=<module at remote 0x7ffff65e6598>)
    at ../Objects/moduleobject.c:544
    #5 0x00007ffff78d5874 in PyImport_Cleanup () at ../Python/import.c:452
    #6 0x00007ffff78e3a38 in Py_FinalizeEx () at ../Python/pylifecycle.c:588
    #7 0x0000000000400795 in main () at /home/smcv/src/dbus-python/test/import-repeatedly.c:19
    (gdb) frame 7
    #7 0x0000000000400795 in main () at /home/smcv/src/dbus-python/test/import-repeatedly.c:19
    19 Py_Finalize();
    (gdb) p i
    $1 = 10

    @smcv smcv mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Aug 11, 2016
    @smcv
    Copy link
    Mannequin Author

    smcv mannequin commented Aug 11, 2016

    This might be a duplicate of https://bugs.python.org/issue24853 but there wasn't enough detail on that bug for me to be sure.

    @ned-deily
    Copy link
    Member

    Thanks for the good test, Simon. Bisection points to b841972ed0bd for bpo-27038 as the culprit here. (It was pushed between 3.6.0a2 and a3.) Brett, Jelle, can you please take a look at this? I'm going to keep it as a Release Blocker for now.

    @brettcannon
    Copy link
    Member

    The revision that Ned found only changed posixmodule.c by adding PyModule_AddObject(m, "DirEntry", (PyObject *)&DirEntryType); at the end of the module initializer. Only thing I can think of is it needs to go into the if (!initialized) block:

    if (!initialized) {
    (maybe as an else clause for the PyType_Ready() call at
    if (PyType_Ready(&DirEntryType) < 0)
    ).

    @zhangyangyu
    Copy link
    Member

    It looks to me from the traceback Simon given that DirEntryType's dealloc function is called. From the code, shouldn't we Py_INCREF DirEntryType before PyModule_AddObject?

    @zhangyangyu
    Copy link
    Member

    Try Simon's snippet I think I am right.

    With the current code, the result is:

    1..1
    Segmentation fault (core dumped)

    With Py_INCREF:

    1..1
    ok 1 - was able to loop 100 times

    I upload the working patch.

    @ned-deily
    Copy link
    Member

    Thanks for the patch. FWIW, that does seem to solve the crash. Brett, look good to you? If so, I can push the fix.

    @brettcannon
    Copy link
    Member

    Patch LGTM.

    @brettcannon brettcannon assigned ned-deily and unassigned brettcannon Aug 15, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 15, 2016

    New changeset a95d98086621 by Ned Deily in branch 'default':
    Issue bpo-27736: Prevent segfault after interpreter re-initialization due
    https://hg.python.org/cpython/rev/a95d98086621

    @ned-deily
    Copy link
    Member

    I've applied the patch for 3.6.0a4 so the immediate issue is fixed. I wonder if it would be worthwhile adapting Simon's test case to add to the embedding test cases in Lib/test/test_capi.py. I'm going to leave the issue open for a while in case someone is interested in doing that.

    @ned-deily ned-deily removed their assignment Aug 15, 2016
    @zhangyangyu
    Copy link
    Member

    We don't have to change Lib/test/test_capi.py. It has already got a repeated init and fini test[1]. All we need to do is increase the loop number[2]. I have tried with 15, the test case fails reliably. And with the previous patch applied, it won't fail.

    [1] https://hg.python.org/cpython/file/tip/Lib/test/test_capi.py#l386
    [2] https://hg.python.org/cpython/file/tip/Programs/_testembed.c#l44

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 16, 2016

    New changeset 4f69626fd923 by Ned Deily in branch 'default':
    Issue bpo-27736: Improve the existing embedded interpreter init/fini test
    https://hg.python.org/cpython/rev/4f69626fd923

    @ned-deily
    Copy link
    Member

    Excellent! Thanks for digging into that. Increasing the iterations seems to work fine within the (understandable) limitations of the existing embedded tests, e.g. only work from a source tree build (not an installed build nor from a separate build directory) etc. Case closed.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants