[Lldb-commits] [PATCH] D81200: [vscode] set default values for terminateDebuggee for the disconnect request

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 2 inline comments as done.
wallace added a comment.

Well, the problem that i've seen happens mostly with long running processes like services that just don't die. So this fixes those issues anyway because those processes are not dying when they should. I tried an older version of the IDE and it sends all the time the terminateDebuggee flag. 
Anyway, what you mention made me notice something. This is the existing flow without this change

> vscode sends a disconnect request. At this point the UI might stop showing the debug session, but it's still running

< lldb-vscode receives it and doesn't terminate the inferior nor terminates itself
< the inferior keeps running and whenever there's output, it's sent to vscode
< if the process terminates, it sends a terminate event

> vscode then finishes the debug session

A problem that I see here is that lldb-vscode was still alive. I'll investigate that tomorrow.

Anyway,wWe still need to investigate why some other processes don't die under normal circumstances, but I'll do it in a different patch.



================
Comment at: lldb/test/API/tools/lldb-vscode/disconnect/main.c:10
+  int x;
+  // We wait for a signal to proceed
+  scanf("%d", &x);
----------------
clayborg wrote:
> This comment seems off? You can call "pause()" to wait for a signal, but that won't work on non unix based systems. Is the comment just left over?
ah yes, it's a left over. I don't want to do something too posix dependant. Good catch


================
Comment at: lldb/tools/lldb-vscode/VSCode.h:92
   bool stop_at_entry;
+  bool is_attach;
   // Keep track of the last stop thread index IDs as threads won't go away
----------------
clayborg wrote:
> This should be initialized in the VSCode::VSCode constructor? Surely we are initializing the other values right?
 oh thank you, i didn't notice that constructor :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81200/new/

https://reviews.llvm.org/D81200