Support terminateThreadsRequest and handle GDB notify after thread group existed

[nguyenbahoangquan]

Currently, when users use terminate threads in Callstack view, this does not work. So, we need to support terminate thread request to handle this case.

[nguyenbahoangquan]

There seems to be some mixed terminology between threads and thread groups in this PR. AFAICT in this change if a user terminates a single thread, the whole process is terminated. Is that the intention? I don't think that GDB support terminating threads, but this may be a limitation of the DAP model.

The option "Terminate thread" is only supported for threads group(when user right click at a thread group in call stack view, not available for child thread). So, we only need to handle for terminate thread group

[nguyenbahoangquan]

There seems to be some mixed terminology between threads and thread groups in this PR. AFAICT in this change if a user terminates a single thread, the whole process is terminated. Is that the intention? I don't think that GDB support terminating threads, but this may be a limitation of the DAP model.

Can you provide a test that verifies this change and review and sign the ECA.

Could you please guide me to write them, I don't have much knowledge about it.

[jonahgraham]

Can you provide a test [...]

Could you please guide me to write them, I don't have much knowledge about it.

Because you are doing multi-threaded changes I would start with src/integration-tests/multithread.spec.ts and adapt as needed. If you need multiple thread groups created that may provide somewhat more of a challenge as cdt-gdb-adapter base code does not yet support that out of the box so you may need to use initCommands or something similar to start multiple processes.

[jonahgraham]

There seems to be some mixed terminology between threads and thread groups in this PR. AFAICT in this change if a user terminates a single thread, the whole process is terminated. Is that the intention? I don't think that GDB support terminating threads, but this may be a limitation of the DAP model.

The option "Terminate thread" is only supported for threads group(when user right click at a thread group in call stack view, not available for child thread). So, we only need to handle for terminate thread group

I don't think I understand this point. With your change, if I launch the MultiThread program I can see this:

To client: {"seq":0,"type":"response","request_seq":1,"command":"initialize","success":true,"body":{"supportsConfigurationDoneRequest":true,"supportsSetVariable":true,"supportsConditionalBreakpoints":true,"supportsHitConditionalBreakpoints":true,"supportsLogPoints":true,"supportsFunctionBreakpoints":true,"supportsDisassembleRequest":true,"supportsReadMemoryRequest":true,"supportsWriteMemoryRequest":true,"supportsTerminateThreadsRequest":true}}

showing the adapter supports terminate which indicates I am running your code properly.

In the UI if I right-click on one of the threads I can see terminate:

and if I select that option I see this in the log:

From client: terminateThreads({"threadIds":[2]}) GDB command: 26 -exec-interrupt --thread 2 GDB command: 27 -interpreter-exec --thread-group i2 console kill To client: {"seq":0,"type":"response","request_seq":12,"command":"terminateThreads","success":true} GDB result: 26 done GDB result: 27 error,msg="Invalid thread group for the --thread-group option"

But there isn't a thread group i2 so the code fails.

The above snippet happens when running in all stop mode, similar in non-stop.

[jonahgraham]

On further reflection, I don't see how to resolve this issue in cdt-gdb-adapter. As far as I know you cannot terminate a single thread in GDB, so I don't think it is appropriate to say "supportTerminateThreadsRequest".

There seems to be an issue in vscode that it is showing "Terminate Thread" even when "supportTerminateThreadsRequest" is false.

Is the correct fix to make vscode only show "Terminate Thread" in UI when it is actually available?

[jonahgraham]

But there isn't a thread group i2 so the code fails.

Note that sometimes the thread group number does existing matching the thread number by coincidence. In the same example of MultiThread, if I terminate main thread I do see it terminating the process. Even in that case though the UI is not updating correctly.

Here is the log when terminating the main thread (thread 1 which happens to match the group i1):

From client: terminateThreads({"threadIds":[1]}) GDB command: 24 -exec-interrupt --thread 1 GDB command: 25 -interpreter-exec --thread-group i1 console kill To client: {"seq":0,"type":"response","request_seq":11,"command":"terminateThreads","success":true} GDB result: 24 done To client: {"seq":0,"type":"event","event":"output","body":{"category":"stdout","output":"Kill the program being debugged? (y or n) [answered Y; input not from terminal]\n"}} Kill the program being debugged? (y or n) [answered Y; input not from terminal] GDB notify async: thread-exited,id="1",group-id="i1" GDB notify async: thread-exited,id="2",group-id="i1" GDB notify async: thread-exited,id="3",group-id="i1" GDB notify async: thread-exited,id="4",group-id="i1" GDB notify async: thread-exited,id="5",group-id="i1" GDB notify async: thread-exited,id="6",group-id="i1" GDB notify async: thread-group-exited,id="i1" To client: {"seq":0,"type":"event","event":"thread","body":{"reason":"exited","threadId":1}} To client: {"seq":0,"type":"event","event":"output","body":{"category":"stdout","output":"[Inferior 1 (process 128626) killed]\n"}} [Inferior 1 (process 128626) killed] GDB result: 25 done From client: stackTrace({"threadId":2,"startFrame":0,"levels":20}) GDB command: 26 -stack-info-depth --thread 2 100 GDB result: 26 error,msg="Invalid thread id: 2" To client: {"seq":0,"type":"response","request_seq":12,"command":"stackTrace","success":false,"message":"Invalid thread id: 2","body":{"error":{"id":1,"format":"Invalid thread id: 2","showUser":true}}}

Here is what the Call Stack view looks like after:

There seems to be some mixed terminology between threads and thread groups in this PR

I am going to restate this from my first review and it is clear to me that this PR is using threads and thread groups interchangeably, but that isn't ok. You can't simply add i to a thread id to get a thread group, and you can't simply remove i from thread group to get thread id.

[jonahgraham]

I think the best way to proceed here is to explain if the requirement is to get thread groups to terminate, or is the issue the "Terminate Thread" showing up in the UI when it isn't applicable.

[jonahgraham]

For now I am going to close this PR. Please create an issue to describe the problem you are facing so we can discuss where the right place for a solution is.