gh-121450: Make inline breakpoints use the most recent pdb instance

[gaogaotiantian]

pdb.set_trace() will use the most recent pdb instance created if there is one. A minor fix in bdb is introduced otherwise the debugger will stop in Bdb.reset(). A small fix in test_pdb is also required because we need to make sure the instance pdb.set_trace is using the patched instance.

The common debugger experience won't be much different to the users. We never explain the implementation details of pdb.set_trace in our documentation so I don't even see a conflict with existing documentation. Not sure if any new documentation is needed at this point.

• Issue: Inline breakpoints should use the most recent pdb isntance, instead of creating a new one #121450

[iritkatriel]

Suggested change

	Inline breakpoints (:func:`breakpoint` and :func:`pdb.set_trace()`) will use the most recent ``Pdb`` instance, instead of creating a new one.
	Inline breakpoints (:func:`breakpoint` and :func:`pdb.set_trace()`) now reuse the same ``Pdb`` instance, instead of creating a new one each time.

[iritkatriel]

Are you sure this can't change behaviour in some cases, where the pdb instance now has a different state? For example, what if the user added breakpoints at the prompt between two breakpoint() calls?

[gaogaotiantian]

This will change the behavior, in favor of the expectation, in some cases, and that's the reason of the change. Adding breakpoints is not impacted because bdb uses a very weird pattern for holding breakpoints. All the breakpoints are stored as class members so every pdb instance can share all the breakpoints - I would guess originally it was designed this way simply to avoid multiple instance issues.

The behavior changes when user is debugging between breakpoint() calls. Now breakpoint() will instantiate a new pdb instance, and lose all the instance specific data the user sets (for example, breakpoint commands, or display). So the user might set a command at a breakpoint like "print(a) when hitting this breakpoint". Then a breakpoint() hit, this command will be discarded forever.

[iritkatriel]

This will change the behavior, in favor of the expectation, in some cases, and that's the reason of the change. Adding breakpoints is not impacted because bdb uses a very weird pattern for holding breakpoints. All the breakpoints are stored as class members so every pdb instance can share all the breakpoints - I would guess originally it was designed this way simply to avoid multiple instance issues.

The behavior changes when user is debugging between breakpoint() calls. Now breakpoint() will instantiate a new pdb instance, and lose all the instance specific data the user sets (for example, breakpoint commands, or display). So the user might set a command at a breakpoint like "print(a) when hitting this breakpoint". Then a breakpoint() hit, this command will be discarded forever.

This could surprise someone, so best explain it in what's new.

[iritkatriel]

what if we're creating this instance while debugging? It would overwrite the instance from the hard coded breakpoint, and this one will be reused at the next hard coded breakpoint. This is not what's intended right?

[iritkatriel]

Maybe Pdb._last_pdb_instance should be set in set_trace().

Probably need a test for this scenario too.

[gaogaotiantian]

You mean in Pdb.set_trace()? That would make some sense, only enlist the instance when it explicitly takes over the tracing.

However, if the user somehow creates and uses a pdb instance before a breakpoint(), we really can't determine the purpose. It could be nested debugging (debug command in pdb which I think we should patch to make sure the _last_pdb_instance is not modified). Or the user could want a new instance, for example, in the test cases.

[iritkatriel]

Not sure I understand. If you set _last_pdb_instance in Pdb.set_trace() then you get clear semantics, and no other breakpoint interrupts with this. Is that not a good thing?

[gaogaotiantian]

No I agree that setting _last_pdb_instance in Pdb.set_trace() is a good approach. And I just realized the nested debugging (debug command in pdb) uses sys.call_tracing so it probably won't affect anything if we do it in Pdb.set_trace(). I'll do the change and see how it works out.

[gaogaotiantian]

And this change actually made the test change unnecessary so it's good. Do you think we need to test more scenarios? I think the case where users explicitly create a Pdb instance in the pdb prompt is really rare - what's the point?

[iritkatriel]

LGTM.

Still needs a what's new entry, I think.

[iritkatriel]

LGTM, add a what's new entry and it's good to go.