threadId inconsistency in continued events

[JohanMabille]

Environment data

• debugpy version: 1.5.1
• OS and version: gentoo 5.4
• Python version (& distribution if applicable, e.g. Anaconda): Python 3.9.7, miniconda
• Using VS Code or Visual Studio: Jupyter Lab + ipykernel

ipykernel is a python process with many threads. One of them is dedicated to execute the code sent by the frontend (JupyterLab), another one is dedicated to communicate with debugpy through a TCP socket.

Actual behavior

When I hit a breakpoint, if I send a continue request, the threadId value contained in the fired continued event is correct (threadId1, which is that of the main thread). However, if I send a next request, the threadId value contained in the fired continued event is wrong (threadId5).

Expected behavior

The threadId value of a continued event should be the same and independent from the request that leads to fire a continue event.

Steps to reproduce:

Launch jupyterlab, open a python notebook and start the debugger. Set a breakpoint in a cell, execute the code. When the code execute the cell, click on the "next" button in the debugger panel (then continue to resume).

Execute the code again, but this time click on continue after the code hit the breakpoint.

In the logs of debugPy, you can notice that the threadId values of the different continued events are different .

[fabioz]

Well, given that the notifications signal that all threads are stopped / all threads are resumed, this shouldn't really make much difference.

-- what's shown right is the first one that really resumed without really taking into account which was signaled as stopped but given that it also signals that allThreadsContinued, it shouldn't make much difference for the client (as all should be marked as continued, the same way that all should be marked as suspended regardless of the thread id because allThreadsStopped was sent on the suspend). I see this more as a shortcoming of the spec (the thread id shouldn't even be needed in this case).

@JohanMabille it is possible to fix it by doing some additional bookkeeping on the debugger, but personally I feel like the current behavior is fine... is this a big issue to you?

[fabioz]

@JohanMabille as a note, I don't think the fix you linked is completely correct because on stopped what it should do is collect all the threads and then consider all of those stopped because allThreadsStopped=true too (so, it should do what the VSCode client does, which is issue a threads request and then mark all of those threads as stopped -- the patch linked above seems to just consider the thread which had the breakpoint added and not other threads which were also stopped...).

So, I'm closing the issue since the spec implementation should be correct here (what's missing is the client handling allThreadsStopped and allThreadsContinued accordingly).

[JohanMabille]

Sorry for the late reply. Thank you for your reply, I think I was mistaken and considered I had this behavior with AllThreadsContinued / AllThreadsStopped set to false. From my understanding, there is no way to specify the debugger behavior for stopping all the threads or only the one which hit the breakpoints (as an argument in the initialize request for instance), the debugger tells you whether it stopped all threads or not when hitting a breakpoint, am I correct?

Also you are right regarding the handling of the stopped event, the implementation is not complete. Thanks for pointing out the Thread request, I totally missed it when I first implemented the DAP in ipykernel.

I agree with you that the current behavior is fine, but I think we should state in the spec that threadId is irrelevant when allThreadsContinued / stopped is true.

[fabioz]

Sorry for the late reply. Thank you for your reply, I think I was mistaken and considered I had this behavior with AllThreadsContinued / AllThreadsStopped set to false. From my understanding, there is no way to specify the debugger behavior for stopping all the threads or only the one which hit the breakpoints (as an argument in the initialize request for instance), the debugger tells you whether it stopped all threads or not when hitting a breakpoint, am I correct?

Yes, right now, in debugpy there's no way to customize the stop/continue (all threads are stopped/resumed at once) -- it is possible to specify the stepping behavior though (through a steppingResumesAllThreads:true/false passed in the launch configuration) -- the default is true.

p.s.: the backend actually supports it, but when using debugpy, it's always configured to multiThreadsSingleNotification: https://github.com/microsoft/debugpy/blob/main/src/debugpy/adapter/clients.py#L272