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

[asvetlov]

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]

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]

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.

[gvanrossum]

Thanks! I will try it out shortly.

[asvetlov]

Rename skip_yield to skipyield

[xdegaye]

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]

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]

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]

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

[xdegaye]

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]

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

[gvanrossum]

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]

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

[gvanrossum]

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

[xdegaye]

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]

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

[gvanrossum]

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]

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]

This version works beautifully in that scenario!

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

[python-dev]

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

[pitrou]

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

[phmc]

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]

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

[pitrou]

Yes, actually, 4f730c045f5f is the culprit.

[python-dev]

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]

I committed a simpler fix.

[python-dev]

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]

Documentation update attached.