don't store exception for identity check

[maxbachmann]

I did look a bit deeper into #1999 and the state was kept alive by State -> Generator -> Frame -> GeneratorExit Exception

Debugpy stores the current exception during unwind for an identity check and only unsets it on the next unwind.

This PR changes this to only store the id which fixes the issue. However to avoid issues on id reuse, this id has to be unset again when a new exception is raised on the thread. This requires having to always listen to monitor.events.RAISE. I don't think that's really a problem but it's something to keep in mind.

@rchiodo do you have a better idea on how we can only hold the exception while required?

I didn't update the generated cython file since I didn't directly know how to do this + this is more of a draft on how this could be solved for now

[rchiodo]

I'm curious why this is suddenly necessary? So you skip handling but then the _thread_local_info will end up different.

[maxbachmann]

What I wanted to add was just the reset of:

        del _thread_local_info.f_unhandled_frame
        del _thread_local_info.f_unhandled_exc_id

to avoid issues with id reuse.

Since prior to my change we only connected to his event if has_caught_exception_breakpoint_in_pydb I skip the rest of the handling in this case.

[rchiodo]

Oh I see. This event is fired more often now. I think it would be better to have a separate event that just does that clearing if 'has_caught_exceptoin_breakpoing_in_pydb' is false. Instead of this logic here which is disconnected from the event being listened to.

[maxbachmann]

Sounds good to me

[maxbachmann]

I did update the PR: I do now generate the cython file.

The second commit uses a separate event handling function. I wasn't completely sure whether this should have just the _clear_unhandled_exception_frame or requires some of the other prior handling.

I don't know enough about the whole system of debugpy to know that this can't lead to a case where:

• _unwind_event -> set id
• _raise_event -> doesn't reset
• _unwind_event -> problem with id reuse

That's also why I initially just added it to the generic _unwind_event just to be sure it definitely gets reset.
So that is something someone with more knowledge about debugpy should determine.

[maxbachmann]

I believe it would just recalculate the top level frame more often than strictly necessary.

[maxbachmann]

Hashing the stacktrace has two downsides:

1. this could presumably clash when throwing the same exception twice from the same location
2. hashing the stacktrace probably isn't cheap either and would be required for every step in the unwind

@fabioz maybe you can add input here since you brought up that the current id handling wouldn't be sufficient to protect against id reuse

[maxbachmann]

Also for reference: My first thought was to unset based on EXCEPTION_HANDLED. However it appears this is actually called before the PY_UNWIND

[rchiodo]

I believe it would just recalculate the top level frame more often than strictly necessary.

I think this is a problem. It's not the same frame then. It's why the frame is cached too. I'm seem to recall fixing some issues around this when we first started using the sys.monitoring stuff and that frame needs to be cached as it's computed when the exception is first unwound.

[maxbachmann]

damn yes then the changes here aren't going to work as is

[rchiodo]

In order to regenerate the cython files you'd run this:

You might need to regenerate the Cython modules after any changes. This can be done by:

Install Python latest (3.14 as of this writing)
pip install cython 'django>=1.9' 'setuptools>=0.9' 'wheel>0.21' twine

On a windows machine:
set FORCE_PYDEVD_VC_VARS=C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Auxiliary\Build\vcvars64.bat
in the pydevd folder: python .\build_tools\build.py

It's in the contributing.md https://github.com/microsoft/debugpy/blob/main/CONTRIBUTING.md#updating-pydevd

[rchiodo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rchiodo]

Oh I forgot one thing. Running the pydevd tests. They're not automated for debugpy. I can try that.

[rchiodo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rchiodo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rchiodo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).