steppingResumesAllThreads with true results in incorrect thread state in the UI

[karthiknadig]

Environment data

• PTVSD version: master
• OS and version: Windows
• Python version (& distribution if applicable, e.g. Anaconda): 3.7
• Using VS Code or Visual Studio: VS Code

Actual behavior

Expected behavior

Show show all threads as running in that case.

Steps to reproduce:

1. Set break point before t.join() and step over that line.

import threading
import time

class RunThread(threading.Thread):
    kind = 'Thread'

    def __init__(self, name, fn, args=()):
        threading.Thread.__init__(self)
        self.name = name
        self.fn   = fn
        self.args = args
        self.start()

    def run(self):
        self.fn(*(self.args))

def th1():
    while True:
        print('th1')
        time.sleep(1)

def th2():
    while True:
        print('th2')
        time.sleep(2)

def th3():
    while True:
        print('th3')
        time.sleep(4)

def main():
    threads = []
    threads.append(RunThread("aaa", th1))
    threads.append(RunThread("bbb", th2))
    threads.append(RunThread("ccc", th3))

    for t in threads:
        t.join()


if __name__ == '__main__':
    main()

[karthiknadig]

We may need to send the continue event in this case, since the threads are continued (may be not as a direct result of user request)? Since stepping is specific to a thread, any other thread that is continued should report via the continued event?

@fabioz @int19h what do you think?

[fabioz]

We actually disable individual hooks in:
(NetCommandFactoryJson.make_thread_run_message and NetCommandFactoryJson.make_thread_suspend_message) and we have the single one sent before/after all threads are resumed in AbstractSingleNotificationBehavior.notify_thread_suspended.

The fix for this would be stop sending the notification from AbstractSingleNotificationBehavior.notify_thread_suspended (just stop sending for the continue) and enable sending the notification on NetCommandFactoryJson.make_thread_run_message.

Still, it seems a bit weird that we send a single suspend notification and multiple continue notifications no?

Is it the case that vscode would like to have notifications sent for each thread while vs would like to have a single for all? Do you think we need another flag for this?

[karthiknadig]

Still, it seems a bit weird that we send a single suspend notification and multiple continue notifications no?

It is weird. VS is the one that needs single suspended event. VSC is more lenient about this. Multiple continued events will definitely break it.

Is it the case that vscode would like to have notifications sent for each thread while vs would like to have a single for all? Do you think we need another flag for this?

I think the clientid should be enough to manage this behavior. so would not need any additional flags. Also, in this particular case VS will not need this continued event at all. VS UI is perfectly happy with the current state reported by the debugger. Since it always assumes all threads should be running while stepping. So no need to make changes for VS.
VSC on the other hand has UI issues. So we should probably send continued events for all threads except the one that is stepping, or alternatively we could also try sending a single continued event with allThreadsContinued=true. We may have to try these out as see if we get the right behavior.

[fabioz]

Actually, it seems that pydevd was already sending the proper continued with allThreadsContinued when all the threads were resumed but the wrapper chose not to send it unless an environment variable named PTVSD_USE_CONTINUED was set (done in 66deeb1 - Turn off sending continued events by default. (#1360)): (and it also wasn't forwarding the allThreadsContinued).

It seems this was done to fix #1361 (although I think that the proper fix would be just forwarding the allThreadsContinued) and #1358 (but we can still not send it for visual studio).

So, the fix I'm thinking about is doing:

    @pydevd_events.handler(pydevd_comm_constants.CMD_THREAD_RESUME_SINGLE_NOTIFICATION)
    def on_pydevd_thread_resume_single_notification(self, seq, args):
        body = args['body']
        if self._client_id != 'visual_studio':
            self.send_event('continued', **body)

What do you think? As a note, I don't know what's the visual studio client id (can you provide it?)

[fabioz]

Actually, I just found the client id in https://microsoft.github.io/debug-adapter-protocol/implementors/tools/