A way to un-breakpoint a set_trace() line

[asmeurer]

This sounds kind of odd, let me explain. I usually enter the debugger with an explicit call to set_trace(). Often, once I've hit that line a few times, I no longer need to stop there, but I don't want to restart the process.

Just as I can manually turn a regular line into a breakpoint with the 'b' command, I would love a way to turn a set_trace() line back into a regular, non-breaking line. The set_trace function would still be called of course, but could examine the calling line to see if it had been marked as non-breaking, and continue.

[asmeurer]

I agree.

[inducer]

👍 Good idea.

[asmeurer]

Thoughts on a GUI for this? I was thinking that whenever a set_trace() is called, it could be added to the breakpoint section (with some kind of indicator that it's not a real breakpoint), and then one could "delete" it from there. It would also highlight the line in the code, probably using a different but similar color to the breakpoint color (falling bak to the breakpoint color if it's not in the theme). Would it be too confusing to use the same key 'b' to handle this? It doesn't make sense to have a breakpoint on a set_trace line, so there shouldn't be ambiguity.

[inducer]

I'm OK even treating it as a "regular" breakpoint with no visual distinction. Since it's impractical to scan the entire source for set_trace lines, we'll have to have each set_trace breakpoint show up once it is first hit.

[nedbat]

I agree: when the set_trace is first hit, make it appear just as any other breakpoint. Then the 'b' command can toggle it off. That sounds great.

[asmeurer]

Yes, source code analysis is far outside the scope of PuDB.

[asmeurer]

Another technical question: should anything be saved to the breakpoints file if a set_trace is "unset"? My initial reaction is no, but it could be annoying if you are restarting the same program many times and your only reason for using set_trace to have PuDB running. #76 gives an alternate idea for this situation, too, though.

[inducer]

Agree with your initial reaction.

[asmeurer]

OK, using some dark magic, I've converted this issue into a pull request.

For now, all I've implemented is the part that lets you skip set_trace, to show that it works. Right now, it just skips after the first time it is called. What remains to be implemented is linking this in with the GUI. Comments on if I'm going about this the right way are appreciated.

[inducer]

I should add: Perhaps use code objects and line numbers as hash keys.

Also, forgot to say thank you for taking a stab at this.

[asmeurer]

Yeah, I figured out a lot more after I pushed that, but I wrote it on an airplane, and just now got to push it. See the commit messages. There is just one thing that doesn't work.

[asmeurer]

So there are two issues

• Only the first set_trace is caught. Take for instance the code

import pudb
def main():
    for i in range(10):
        pudb.set_trace()
        print('test')

    for i in range(5):
        pudb.set_trace()
        print('test2')

main()

If you run this code, it will stop at the first set_trace, and it will mark it red. The second one will not be marked red (this is all correct so far, because we can only mark a set_trace as a breakpoint once it is hit). If you press c it will stop at the set_trace. If you move the line to that set_trace and press b, it will unhighlight it, and if you press c, it will go to the second set_trace. This is where it goes wrong. It doesn't mark the second one red at this point. However, the behavior is correct. If you press c, it will stop at it again, but if you highlight it and press b and then press c, it will exit the script.

So it is being marked as a breakpoint. The issue is that screen is not being redrawn. I couldn't figure out how to communicate this information to the GUI from set_trace.

• I haven't added any of the set_trace calls to the breakpoint list on the right. I'm not sure what this should look like. I feel like they should be listed there, because it would be useful to have a list of all known set_trace calls, but they should be presented differently somehow (in particular, they cannot be deleted or changed, and the condition stuff, though it could be implemented, currently isn't).

[inducer]

(I've split out the issue about improving the breakpoints view.)

[asmeurer]

OK, so I'll do that separately. I feel like even adding set_trace should be done there too, since it will require some refactoring there anyway.

But the first issue is still a blocker. How can I make the UI refresh when set_trace() is called?

[asmeurer]

I tried the new ctrl-l you implemented, but that didn't work. ctrl-l should perhaps invalidate the UI in addition to clearing the screen.

That doesn't exactly solve my problem, though, but I wanted to see if it would work.

[inducer]

ctrl-l invalidating the UI: not a fan.

[asmeurer]

OK, I got bored on the plane today and decided to finish this up. Take a look if my method of resetting the UI is correct (my most recent commit). As far as I know, this is ready to be reviewed and merged.

Basic behavior (in case you forgot): when set_trace() is run, it is treated as a "pseudo-breakpoint". It doesn't show up in the breakpoints list (maybe it should), but it is highlighted with the breakpoint color. You can then select it in the source and press b to "unset" it. From there on out, calls to that set_trace() will be ignored. Obviously, it can't highlight a set_trace() call until it is run.

[inducer]

If I Ctrl-C in the middle of execution with your patch applied, there's a spurious (and confusing) breakpoint shown in the source. (Easy to reproduce with debug_me.py in the source.)

[asmeurer]

That was easy enough to fix, but there seems to be another issue that comes up when there is a normal breakpoint in the same file. I'm still working on that.

[asmeurer]

OK, I fixed that bug, again on an airplane (this time with some pretty hard-core non-PuDB debugging). Lesson learned: be careful with +=. If you use it on on a list, and that list is referenced somewhere else, it will screw up the other code, and is _very_ hard to track down.

[inducer]

In b979fc5, something is still amiss. Here's a reproducer:

1. Add a pu.db in line 33of debug_me.py.
2. Run ./try-the-debugger.sh
3. Hit n 4 times
4. Hit c.
5. When the pu.db is hit, a spurious breakpoint appears on line 36.

[inducer]

Forgot to say: Thanks for your work on this!

[asmeurer]

Doesn't reproduce for me. Does it actually break at the spurious breakpoint?

[asmeurer]

By the way, I noticed that the fake breakpoints are a little off because normal breakpoints stop on the line itself, but set_trace stops on the line after. But I think the fix here is to actually go in and make the set_trace breakpoints color differently.

[asmeurer]

Oh I think I might be seeing what you are seeing now. It only seems to happen in Python 2. Except for me the spurious breakpoint is on line 34 (right below the pu.db).

[asmeurer]

Actually, I think the thing I described is the source of the problem. It thinks the line number is the line after the set_trace (which could be any line depending on the flow of the code). Any thoughts on how to fix this?

[asmeurer]

And why is it Python 2 only? Did Python 3 change the behavior of frame.f_lineno?

[asmeurer]

Searching around, I found some vaguely similar things in the Python issue tracker, like http://bugs.python.org/issue17697#msg186637. But I haven't dug deep enough to know if they are the same, or how to fix this issue. Again, if you have any thoughts, I'd love to hear them.

[asmeurer]

Hmm, now I can only seem to reproduce it in Python 3.

[asmeurer]

OK, I have applied the patch from https://bugs.python.org/issue16482, which was mentioned in https://bugs.python.org/issue17697 to fix the problem, and it seems to work (and based on my understanding of the problem, it seems like it should work). It looks like this is a workaround for a bug in the interpreter itself, where line numbers can be reported incorrectly on frames with traces set on them when the global trace is removed. The workaround, as I understand it, is to remove the traces when continuing so that the line numbers get reported correctly. As such this probably deserves some testing. Everything seems to work in my limited testing. I can definitely reproduce your issue and this latest commit fixes it.

Also see https://bitbucket.org/xdegaye/pdb-clone/commits/123d1b6db6649f4b54712db321865fda55395488, which looks like it goes even further than my patch to toward working around this issue (see also the current code). I'm unclear why this fix would be necessary for anything other than continue, however, and it seems to me that the worst that can happen is that another behavior might lead to an incorrect line number being reported somewhere.

At any rate, please test this. To my knowledge, it should be ready to go (again).

[inducer]

In my limited testing, this seems to do the trick. This whole 'disable set_trace' thing is pretty slick, I like it. Ready to go in as far as I am concerned.

[asmeurer]

OK, let's do it.

[inducer]

Cool, thanks!