gh-58956: Fix a frame refleak in bdb

[gaogaotiantian]

We added self.enterframe in bdb.Bdb, but that will leak the frame reference of the enterframe until the next time the debugger is triggered. As the frame is a linked list, one leaked frame will also leak many others. We should fix this.

This is not a bug fix per se, but this blocks #125549, which is a backport for a bug fix. So we should backport this to 3.12 and 3.13, then merge the bug fix in #125549 to avoid leaking frames.

• Issue: Pdb does not stop at a breakpoint #58956

[iritkatriel]

Should this be in a finally block?

[gaogaotiantian]

Theoretically, it's safer to put it in a try finally block. I did not do it for two reasons:

1. Currently there's no exception trigger in the code. We'll have a much bigger issue if there is an unexpected exception here.
2. This is a "not a bug fix" code that we want to backport, so I want to make it as trivial as possible.

I suggest that we do it in the future when there is a possible exception trigger. Or at least after the backport.

[iritkatriel]

Not sure I see how putting this in a finally makes any difference w.r.t to backporting?

[gaogaotiantian]

The code diff would be a bit more and we would have an extra indentation.

Currently, all the finally in bdb/pdb protect exceptions from user code, not the debugger code. If we set a precedence to assume every line of code can generate an exception and we have to make sure everything, including references, needs to be properly released, we would be in a giant hole.

For example, we are holding a frame reference in self.returnframe, which is set by _set_stopinfo. And the way we release it, is when we do a continue, set_continue would set it to None. But what if there's a random exception somewhere? Do we protect that with a finally?

We are also using the frame as a key in self.frame_trace_line_opcodes, thus keeping a reference to it, which will also be released in set_continue as long as there are no breakpoints, do we protect those as well (it's right before the next change)? If so, we have two resources to release, do we do

try:
    ...
finally:
    try:
        self.frame_trace_lines_opcodes = {}
    finally:
        self.enterframe = None

I think my point is - the debugger should not crash. If it crashes by itself, we don't really care about reference leaks, we should fix the debugger. Therefore, we should only care about exceptions from the user code and make sure resources are released in those cases.

[iritkatriel]

self.frame_trace_lines_opcodes can be cleared in the same finally. Anyway, up to you.

[gaogaotiantian]

Right, we normally assume self.frame_trace_lines_opcodes = {} won't raise an exception - that's why we can put them together in the same finally. I think we should do the same for most of the internal debugger code - unless it's very critical. That's how pdb/bdb does it for now. It assumes the debugger code works (at least the simple ones) and release resources accordingly.

In set_trace, the code between setting enter_frame and releasing enter_frame are basically all assignments (including set_stepinstr), so we can assume that no exception will be raised (otherwise self.frame_trace_lines_opcodes = {} is not safe either).

As for set_continue, we actually can't prevent the leak from happening if there's a crash in the debugger, because that means it's possible to break out of the debugger without running set_continue. We now assume the user always gets a chance to run continue, which would release the reference - that might not be the case if we consider the possibility of a debugger crash. Then our protection to ensure the release happens in set_continue does not do much.