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

Skip stack unwinding when "next", "until" and "return" pdb commands executed in generator context #60800

Closed
asvetlov opened this issue Dec 2, 2012 · 27 comments
Assignees
Labels
release-blocker stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@asvetlov
Copy link
Contributor

asvetlov commented Dec 2, 2012

BPO 16596
Nosy @gvanrossum, @birkenfeld, @pitrou, @larryhastings, @asvetlov, @xdegaye, @phmc
Files
  • issue16596.diff
  • issue16596_v2.diff
  • issue16596_nostate.diff
  • issue16596_nostate_2.diff
  • issue16596_nostate_3.diff
  • issue16596_nostate_4.diff
  • issue16596_leak.diff: Fix for ref leak in committed patch
  • pdb_doc.diff
  • 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/gvanrossum'
    closed_at = <Date 2013-11-23.13:13:35.797>
    created_at = <Date 2012-12-02.23:23:04.743>
    labels = ['type-feature', 'library', 'release-blocker']
    title = 'Skip stack unwinding when "next",  "until" and "return" pdb commands executed in generator context'
    updated_at = <Date 2014-03-10.10:12:38.006>
    user = 'https://github.com/asvetlov'

    bugs.python.org fields:

    activity = <Date 2014-03-10.10:12:38.006>
    actor = 'xdegaye'
    assignee = 'gvanrossum'
    closed = True
    closed_date = <Date 2013-11-23.13:13:35.797>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2012-12-02.23:23:04.743>
    creator = 'asvetlov'
    dependencies = []
    files = ['28189', '28205', '28225', '28280', '32699', '32735', '32794', '34326']
    hgrepos = []
    issue_num = 16596
    keywords = ['patch']
    message_count = 27.0
    messages = ['176815', '176817', '176862', '176898', '177050', '177051', '177059', '177319', '177344', '202991', '202999', '203309', '203312', '203350', '203352', '203423', '203537', '203543', '203668', '203986', '203992', '203995', '203996', '204001', '204002', '213008', '213022']
    nosy_count = 8.0
    nosy_names = ['gvanrossum', 'georg.brandl', 'pitrou', 'larry', 'asvetlov', 'xdegaye', 'python-dev', 'pconnell']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16596'
    versions = ['Python 3.4']

    @asvetlov
    Copy link
    Contributor Author

    asvetlov commented Dec 2, 2012

    GvR in http://mail.python.org/pipermail/python-ideas/2012-November/017991.html has requested for improving pdb to jump over yield instead of following it.

    @asvetlov asvetlov added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Dec 2, 2012
    @asvetlov
    Copy link
    Contributor Author

    asvetlov commented Dec 2, 2012

    Patch attached.
    It modifies "next", "return" and "until" commands behavior when they are executed if current debugged function is generator.

    1. "next" skips stack unwindind if yield tries to follow it. Now it goes just to next executed line.
    2. "until" does the same: skips all yields until target is reached.
    3. "return" waits to return *from* generator (by explicit or implicit StopIteration or GeneratorExit exception) ignoring following yields inside generator.

    @asvetlov asvetlov changed the title Skip stack unwinding when "next", "until" and "return" pdb commands executed Skip stack unwinding when "next", "until" and "return" pdb commands executed in generator context Dec 2, 2012
    @gvanrossum
    Copy link
    Member

    Thanks! I will try it out shortly.

    @asvetlov
    Copy link
    Contributor Author

    asvetlov commented Dec 4, 2012

    Rename skip_yield to skipyield

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Dec 6, 2012

    In the test named 'test_pdb_return_command_for_generator' in the patch, the
    return command does not cause pdb to stop at the StopIteration debug event as
    expected. Instead the following step command steps into the generator.

    With the patch applied, in the following debugging session, pdb does not stop
    after the 'next' command. Pdb should stop to stay consistent with the way
    'next' behaves in the same case on a plain function.

    === bar.py ===

    def g():
        yield 0
    
    it = g()
    while True:
        try:
            x = next(it)
            print(x)
        except StopIteration:
            break

    ==================

    $ ./python -m pdb /tmp/bar.py
    > /tmp/bar.py(1)<module>()
    -> def g():
    (Pdb) break g
    Breakpoint 1 at /tmp/bar.py:1
    (Pdb) continue
    > /tmp/bar.py(2)g()
    -> yield 0
    (Pdb) next
    0
    The program finished and will be restarted
    > /tmp/bar.py(1)<module>()
    -> def g():
    (Pdb)

    ==================

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Dec 6, 2012

    This new patch fixes the two problems described in my previous message.
    The patch is different from Andrew's patch in that it does not use a new state
    variable, and the test cases in the patch are a copy of Andrew's patch except
    for test_pdb_return_command_for_generator.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Dec 6, 2012

    When the generator is used in a for loop, the interpreter handles the
    StopIteration in its eval loop, and the exception is not raised. So it may be
    considered as confusing to have pdb behave differently with a generator
    depending on its context. A way to fix this would be to not ignore the return
    debug events, with the drawback of a more verbose debug process with
    generators.

    @gvanrossum
    Copy link
    Member

    It looks like xdegaye's patch breaks 'n' when not debugging a generator.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Dec 11, 2012

    The 'until' command is also broken (by xdegaye's patch) when issued at a return
    debug event and not debugging a generator.

    This new patch fixes both problems.

    The patch also adds another test case to check that pdb stops after a 'next',
    'until' or 'return' command has been issued at a return debug event.

    @gvanrossum
    Copy link
    Member

    I'd love it if someone could review this. This would be a great improvement to debugging coroutines in asyncio.

    @gvanrossum
    Copy link
    Member

    I think this is not ready for inclusion. It works wonderfully when stepping over a yield[from], but I can't seem to get it to step nicely *out* of a generator. (Details on request -- basically I put a "pdb.set_trace()" call in Tulip's fetch3.py example and step around.)

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Nov 18, 2013

    A description of what goes wrong when stepping out of the generator
    would be helpful.

    @gvanrossum
    Copy link
    Member

    Basically the debugger lost control and the program ran to completion after I hit 'n' that returned from the coroutine.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Nov 19, 2013

    This is a consequence of the problem mentioned in msg 177059 above.

    New patch 'issue16596_nostate_3.diff' fixes both problems by having the interpreter
    issue an exception debug event when processing a StopIteration in target FOR_ITER:

    • The same debug events are issued now, wether the generator is run within a for loop or not.
    • 'n' stops now at both the return and exception debug events on returning from the generator.

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Nov 19, 2013

    Forgot to say that the only difference between this patch and the previous one is in Python/ceval.c.

    @gvanrossum
    Copy link
    Member

    It's not fixed. Let me paste in a session. This uses the latest Tulip repo (simple_tcp_server.py was just added). I've added "import pdb; pdb.set_trace()" to the top of the client() coroutine, to set a breakpoint (I'm a very unsophisticated pdb user :-). I step over a yield-from, great. Then I step into recv(). Note the final 'n' command. This is at the return statement in recv(). At this point I expect to go back into the client() coroutine, but somehow the debugger loses control and the program finishes execution without giving control back.

    bash-3.2$ ~/cpython/python.exe -m examples.simple_tcp_server
    ~/cpython/python.exe -m examples.simple_tcp_server

    /Users/guido/tulip/examples/simple_tcp_server.py(119)client()
    -> reader, writer = yield from asyncio.streams.open_connection(
    (Pdb) n
    n
    /Users/guido/tulip/examples/simple_tcp_server.py(120)client()
    -> '127.0.0.1', 12345, loop=loop)
    (Pdb)

    /Users/guido/tulip/examples/simple_tcp_server.py(122)client()
    -> def send(msg):
    (Pdb)

    /Users/guido/tulip/examples/simple_tcp_server.py(126)client()
    -> def recv():
    (Pdb)

    /Users/guido/tulip/examples/simple_tcp_server.py(132)client()
    -> send("add 1 2")
    (Pdb)

    add 1 2
    /Users/guido/tulip/examples/simple_tcp_server.py(133)client()
    -> msg = yield from recv()
    (Pdb) s
    s
    --Call--
    /Users/guido/tulip/examples/simple_tcp_server.py(126)recv()
    -> def recv():
    (Pdb) n
    n
    /Users/guido/tulip/examples/simple_tcp_server.py(127)recv()
    -> msgback = (yield from reader.readline()).decode("utf-8").rstrip()
    (Pdb) n
    n
    /Users/guido/tulip/examples/simple_tcp_server.py(128)recv()
    -> print("< " + msgback)
    (Pdb) n
    n
    < 3.0
    /Users/guido/tulip/examples/simple_tcp_server.py(129)recv()
    -> return msgback
    (Pdb) n
    n
    repeat 5 hello
    < begin
    < 1. hello
    < 2. hello
    < 3. hello
    < 4. hello
    < 5. hello
    < end
    client task done: Task(<_handle_client>)<result=None>
    bash-3.2$

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Nov 20, 2013

    Hopefully issue16596_nostate_4.diff should fix this.
    The patch issues a StopIteration debug event in ceval.c (similar to the change made in the previous
    patch for the for loop), when the subgenerator is exhausted. This debug event is printed as
    'Internal StopIteration' by pdb to indicate that it is not a real user exception. Two tests have
    been added: test 'next' when returning from a generator in a for loop and 'test' next when returning
    from a subgenerator.

    @gvanrossum
    Copy link
    Member

    This version works beautifully in that scenario!

    Does anyone else reading this bug report object to this being committed?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 21, 2013

    New changeset 95eea8624d05 by Guido van Rossum in branch 'default':
    Better behavior when stepping over yield[from]. Fixes bpo-16596. By Xavier de Gaye.
    http://hg.python.org/cpython/rev/95eea8624d05

    @gvanrossum gvanrossum self-assigned this Nov 21, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Nov 23, 2013

    This commit created a reference leak:

    ./python -m test -R 3:2 test_trace
    [1/1] test_trace
    beginning 5 repetitions
    12345
    .....
    test_trace leaked [128, 128] references, sum=256

    @pitrou pitrou reopened this Nov 23, 2013
    @phmc
    Copy link
    Mannequin

    phmc mannequin commented Nov 23, 2013

    It looks like call_exc_trace is leaking refs to Py_None.

    I believe the attached patch fixes the issue (it certainly fixes Antoine's failing invokation :)

    @phmc
    Copy link
    Mannequin

    phmc mannequin commented Nov 23, 2013

    Full run of the test suite was clean, so the fix is ready to go.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 23, 2013

    Yes, actually, 4f730c045f5f is the culprit.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 23, 2013

    New changeset 8f556ee0f6ba by Antoine Pitrou in branch 'default':
    Fix refleak introduced by 4f730c045f5f (issue bpo-18408) and unveiled by 95eea8624d05 (issue bpo-16596).
    http://hg.python.org/cpython/rev/8f556ee0f6ba

    @pitrou
    Copy link
    Member

    pitrou commented Nov 23, 2013

    I committed a simpler fix.

    @pitrou pitrou closed this as completed Nov 23, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 10, 2014

    New changeset 5c6c96c82afb by R David Murray in branch 'default':
    whatsnew: pdb works for debugging asyncio programs (bpo-16596).
    http://hg.python.org/cpython/rev/5c6c96c82afb

    @xdegaye
    Copy link
    Mannequin

    xdegaye mannequin commented Mar 10, 2014

    Documentation update attached.

    @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
    release-blocker stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants