Uh oh!
There was an error while loading. Please reload this page.
Fix duplicate stopped event when two threads hit a breakpoint at once - #2056
Fix duplicate stopped event when two threads hit a breakpoint at once#2056Alexandre Perez (aperez) wants to merge 3 commits into
Conversation
Suspending the other threads marks them `CMD_THREAD_SUSPEND`, which is what lets the single notification drop their redundant stopped events. But a line event checks breakpoints before it checks for a pending suspend, so a thread suspended while blocked still reports its own breakpoint on waking, overwriting that reason and emitting a second stopped event. Leave an already suspended thread's reason alone. It then keeps the reason it first stopped for, which is invisible over DAP and shows only on the legacy protocol with a suspend-all breakpoint policy.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Rich Chiodo (rchiodo)
commented
Aug 5, 2026
🔒 Automated review in progress — @rchiodo is auto-reviewing this PR. |
| print("main") # @bp1 | ||
| thread.join() | ||
| with debug.Session() as session: |
There was a problem hiding this comment.
Issue · Please address or respond
None means the debuggee is not expected to exit, but this launch-mode debuggee exits with code 0 after disconnect(), causing teardown to fail with assert 0 == None. Use the harness's any-integer expectation (some.int, importing it from tests.patterns) instead.
There was a problem hiding this comment.
I am using expected_exit_code = some.int now.
`expected_exit_code = None` means the debuggee is not expected to exit at all, but this one is launched and does exit once `disconnect()` releases it, so the `exited` event trips `assert self.exit_code == self.expected_exit_code`. Use `some.int`, as `test_disconnect` does for the same disconnect-while-stopped shape.
Rich Chiodo (rchiodo)
commented
Aug 5, 2026
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Rich Chiodo (rchiodo)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Rich Chiodo (rchiodo)
commented
Aug 5, 2026
It looks like this is breaking django/flask breakpoints: These two tests are continually failing for all os/versions: test_django_template_exception_no_multiproc[launch] |
Ah , I wasn't running the django and flask tests while working on this fix. Let me edit the fix so it does not regress those tests. |
The guard added earlier also blocked genuine re-stops. A Django template exception suspends the thread with `CMD_ADD_EXCEPTION_BREAK`, then the caught-exception handler re-suspends it with `CMD_STEP_CAUGHT_EXCEPTION`. Both map to an "exception" stopped event, so the stop itself looked right and only `exceptionInfo` noticed, reporting breakMode "unhandled" instead of "always". Restrict the guard to threads still carrying `CMD_THREAD_SUSPEND`, which is the side-effect suspend the single notification needs to drop. Covered by the existing test_django template exception tests.
Alexandre Perez (aperez)
commented
Aug 5, 2026
@rchiodo latest version filters by stop reason to make sure we're only including suspends. |
Rich Chiodo (rchiodo)
commented
Aug 5, 2026
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
| # a reason of its own yet, and the single notification behavior keys off that reason | ||
| # to drop its redundant stopped event. Any other reason is a real stop, so it must | ||
| # still be allowed to overwrite an earlier one. | ||
| if info.pydev_state != STATE_RUN and getattr(thread, "stop_reason", None) == CMD_THREAD_SUSPEND: |
There was a problem hiding this comment.
Warning · Non-blocking recommendation
This intentionally fixes the duplicate at the shared write rather than reordering the line-event handler that evaluates breakpoints before pending suspends. Please add an explicit issue/reference for the deferred ordering fix so that root cause remains tracked.
| # A thread suspended only as a side effect of another thread's stop has not reported | ||
| # a reason of its own yet, and the single notification behavior keys off that reason | ||
| # to drop its redundant stopped event. Any other reason is a real stop, so it must | ||
| # still be allowed to overwrite an earlier one. |
There was a problem hiding this comment.
Warning · Non-blocking recommendation
This early return skips the remaining suspend-type, stepping, and stop-state updates for every caller that re-marks an already suspended thread with CMD_THREAD_SUSPEND, not only this concurrent-breakpoint path. Please audit those callers; if any legitimately update those fields, narrow the guard to preserve only the stop reason.
Rich Chiodo (rchiodo)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Suspending other threads marks them
CMD_THREAD_SUSPEND, which is what lets the pydevd single notification strategy drop their redundant stopped events. But a line event checks breakpoints before it checks for a pending suspend, so a thread suspended while blocked still reports its own breakpoint on waking, overwriting that reason and emitting a second stopped event.Leave an already suspended thread's reason alone. It then keeps the reason it first stopped for, which is invisible over DAP and shows only on the legacy protocol with a suspend-all breakpoint policy.