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

deeply nested itertools objects segfault #58218

Closed
alex opened this issue Feb 14, 2012 · 25 comments
Closed

deeply nested itertools objects segfault #58218

alex opened this issue Feb 14, 2012 · 25 comments
Assignees
Labels
extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@alex
Copy link
Member

alex commented Feb 14, 2012

BPO 14010
Nosy @arigo, @birkenfeld, @rhettinger, @amauryfa, @benjaminp, @ezio-melotti, @alex, @serhiy-storchaka
Files
  • iter_recursion.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/rhettinger'
    closed_at = <Date 2019-08-24.04:43:08.908>
    created_at = <Date 2012-02-14.15:58:25.896>
    labels = ['extension-modules', 'interpreter-core', 'type-crash']
    title = 'deeply nested itertools objects segfault'
    updated_at = <Date 2019-08-24.04:43:08.907>
    user = 'https://github.com/alex'

    bugs.python.org fields:

    activity = <Date 2019-08-24.04:43:08.907>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2019-08-24.04:43:08.908>
    closer = 'rhettinger'
    components = ['Extension Modules', 'Interpreter Core']
    creation = <Date 2012-02-14.15:58:25.896>
    creator = 'alex'
    dependencies = []
    files = ['29483']
    hgrepos = []
    issue_num = 14010
    keywords = ['patch']
    message_count = 25.0
    messages = ['153344', '153353', '175242', '184631', '184632', '184633', '184660', '185140', '186141', '186143', '186144', '186145', '186147', '186153', '186156', '186157', '186158', '186159', '186161', '199738', '231490', '231535', '246594', '293208', '350347']
    nosy_count = 9.0
    nosy_names = ['arigo', 'georg.brandl', 'rhettinger', 'amaury.forgeotdarc', 'benjamin.peterson', 'ezio.melotti', 'alex', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue14010'
    versions = ['Python 2.7', 'Python 3.3', 'Python 3.4', 'Python 3.5', 'Python 3.6']

    @alex
    Copy link
    Member Author

    alex commented Feb 14, 2012

    http://paste.pocoo.org/show/550884/ will reliably segfault Python3 on all platforms (similar versions for Python2 using itertools work)

    @alex alex added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 14, 2012
    @ezio-melotti ezio-melotti added the type-crash A hard crash of the interpreter, possibly with a core dump label Feb 14, 2012
    @arigo
    Copy link
    Mannequin

    arigo mannequin commented Feb 14, 2012

    The issue is a stack exhaustion. Examples can be trivially made for any iterator that takes another iterator as argument: itertools.takewhile(), zip() in Python3, etc. etc.

    It's just one of many places where CPython does a recursion without checking the recursion depth. CPython still works, based on the resonable assumption that doing such a recursion here is obscure.

    Someone seriously bored could start with some C-based callgraph builder; or alternatively use PyPy, which finds such recursions automatically in its own source, and compare all places where a recursion check is inserted with the corresponding place in CPython. There are a large number of them (761, not counting the JIT), so be patient :-(

    @alex
    Copy link
    Member Author

    alex commented Nov 9, 2012

    Since the paste is dead:

    i = filter(bool, range(5))
    for _ in range(1000000):
        i = filter(bool, i)
    
    for p in i:
        print(p)

    @serhiy-storchaka
    Copy link
    Member

    I'm trying to solve this issue (it seemed easy), but the bug is worse than expected. Python crashed even without iteration at all.

    it = 'abracadabra'
    for _ in range(1000000):
        it = filter(bool, it)

    del it

    And fixing a recursive deallocator is more harder than iterator.

    What can we do if a deallocator raises RuntimeError due to maximum recursion depth exceeded.

    @amauryfa
    Copy link
    Member

    Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END macros can help:
    http://hg.python.org/cpython/file/57c6435ca03b/Python/traceback.c#l44

    @serhiy-storchaka
    Copy link
    Member

    Thank you. Now I understand why this issue not happened with containers.

    @serhiy-storchaka
    Copy link
    Member

    Here is a patch which adds recursion limit checks to builtin and itertools recursive iterators.

    @serhiy-storchaka serhiy-storchaka added the extension-modules C modules in the Modules dir label Mar 19, 2013
    @amauryfa
    Copy link
    Member

    • The tests with "range(1000000)" seems to duplicate those with recursion limit.
    • zip_iter should would be simpler with a "goto error;"

    LGTM otherwise.

    @rhettinger rhettinger self-assigned this Mar 24, 2013
    @serhiy-storchaka
    Copy link
    Member

    • The tests with "range(1000000)" seems to duplicate those with recursion limit.

    Oh, I forgot to remove old tests when had moved them to special test class.

    • zip_iter should would be simpler with a "goto error;"

    Indeed. Thank you.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 6, 2013

    New changeset aaaf36026511 by Serhiy Storchaka in branch '3.3':
    Issue bpo-14010: Fix a crash when iterating or deleting deeply nested filters
    http://hg.python.org/cpython/rev/aaaf36026511

    New changeset 846bd418aee5 by Serhiy Storchaka in branch 'default':
    Issue bpo-14010: Fix a crash when iterating or deleting deeply nested filters
    http://hg.python.org/cpython/rev/846bd418aee5

    @rhettinger
    Copy link
    Contributor

    This patch didn't have my sign-off. Applying it was premature. It is a somewhat heavy handed fix that slows all the common cases at the expense of an exotic case.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 6, 2013

    New changeset d17d10c84d27 by Serhiy Storchaka in branch '2.7':
    Issue bpo-14010: Fix a crash when iterating or deleting deeply nested filters
    http://hg.python.org/cpython/rev/d17d10c84d27

    @serhiy-storchaka
    Copy link
    Member

    Oh, shame on me. Do I have to immediately revert patches or wait for your post-commit review?

    @rhettinger
    Copy link
    Contributor

    I would appreciate it if you would please revert this patch.

    We need to search for a solution that isn't as fine grained (i.e. not doing increments, decrements, and tests on every single call to iter_next). Ideally, the checks can be confined to the iterator constructor and to dealloc. Or you could try to create some general purpose stack overflow protection that periodically makes sure there is enough stack remaining for C Python to function correctly.

    @amauryfa
    Copy link
    Member

    amauryfa commented Apr 6, 2013

    Or you could try to create some general purpose stack overflow
    protection that periodically makes sure there is enough stack remaining
    for C Python to function correctly.

    Isn't it exactly what Py_EnterRecursiveCall does?

    @benjaminp
    Copy link
    Contributor

    It has no notion of how big the C stack is.

    2013/4/6 Amaury Forgeot d'Arc <report@bugs.python.org>:

    Amaury Forgeot d'Arc added the comment:

    > Or you could try to create some general purpose stack overflow
    > protection that periodically makes sure there is enough stack remaining
    > for C Python to function correctly.

    Isn't it exactly what Py_EnterRecursiveCall does?

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue14010\>


    @rhettinger
    Copy link
    Contributor

    Isn't it exactly what Py_EnterRecursiveCall does?

    No, it isn't. Py_EnterRecursiveCall() counts calls and measures depth. It is sprinked all over the source code, everywhere a potentially recursive call could be made.

    Instead, it would be nice if the interpreter could monitor the actual stack size and take appropriate actions when it is running low on space. The would save us from putting in expensive fine grained checks throughout the source code.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 6, 2013

    New changeset e07e6d828150 by Serhiy Storchaka in branch '2.7':
    Revert a premature patch for issue bpo-14010 (changeset d17d10c84d27).
    http://hg.python.org/cpython/rev/e07e6d828150

    New changeset 7b75f0bd9a5e by Serhiy Storchaka in branch '3.3':
    Revert a premature patch for issue bpo-14010 (changeset aaaf36026511).
    http://hg.python.org/cpython/rev/7b75f0bd9a5e

    New changeset 504eed5a82a3 by Serhiy Storchaka in branch 'default':
    Revert a premature patch for issue bpo-14010 (changeset 846bd418aee5).
    http://hg.python.org/cpython/rev/504eed5a82a3

    @serhiy-storchaka
    Copy link
    Member

    I apologize for my negligence.

    @birkenfeld
    Copy link
    Member

    See bpo-14507 for another instance of this in starmap().

    @serhiy-storchaka
    Copy link
    Member

    See bpo-22911 for another instance of this in chain.from_iterable().

    @ethanfurman
    Copy link
    Member

    From Terry Reedy in bpo-22920:
    ------------------------------
    Ian Kelly (python-list, version unspecified) got "Segmentation fault (core dumped)". With 2.7, 3.4.2, 3.5, I get same in interactive interpreter, the Windows "python has stopped working" box from console, or subprocess hang with Idle.

    @serhiy-storchaka
    Copy link
    Member

    See bpo-24606 for another instance of this in map().

    @serhiy-storchaka
    Copy link
    Member

    See bpo-30297 for yet one instance of this in starmap().

    @pppery pppery mannequin changed the title deeply nested filter segfaults deeply nested itertools objects segfault Jan 28, 2018
    @rhettinger
    Copy link
    Contributor

    [Armin]

    It's just one of many places where CPython does a recursion
    without checking the recursion depth. CPython still works,
    based on the resonable assumption that doing such a recursion
    here is obscure.

    I agree with that assessment and am going to close this as something we can live with (or least have lived with successfully for a long time). AFAICT, this situation doesn't arise in practical code.

    It is possible to slow down the language by adding a variant of recursion checks to every call to an iterator. But this just makes the language pay a code complexity cost and performance cost for something that doesn't really affect real users. People typically choose itertools for their speed (otherwise, plain generators can be clearer). We shouldn't work against the needs of those users.

    A robust, clean, and performant solution wouldn't be easy but would likely involve some general purpose stack overflow protection that periodically makes sure there is enough stack remaining for C Python to function correctly.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir 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

    8 participants