Author: Walter Erquinigo
Date: 2020-06-23T11:47:43-07:00
New Revision: 0a9e7d0b6befad866dfd61f05b774247e0867121

URL: 
https://github.com/llvm/llvm-project/commit/0a9e7d0b6befad866dfd61f05b774247e0867121
DIFF: 
https://github.com/llvm/llvm-project/commit/0a9e7d0b6befad866dfd61f05b774247e0867121.diff

LOG: [vscode] set default values for terminateDebuggee for the disconnect 
request

Summary:
Recently I've noticed that VSCode sometimes doesn't send the terminateDebuggee 
flag within the disconnectRequest,
even though lldb-vscode sets the terminateDebuggee capability correctly.
This has been causing that inferiors don't die after the debug session ends, 
and many users have reported issues because of this.

An easy way to mitigate this is to set better default values for the 
terminateDebuggee field in the disconnect request.
I'm assuming that for a launch request, the default will be true, and for 
attach it'll be false.

Reviewers: clayborg, labath, aadsm

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D81200

Added: 
    lldb/test/API/tools/lldb-vscode/disconnect/Makefile
    lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
    lldb/test/API/tools/lldb-vscode/disconnect/main.cpp

Modified: 
    lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
    lldb/tools/lldb-vscode/VSCode.cpp
    lldb/tools/lldb-vscode/VSCode.h
    lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 
    


################################################################################
diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 790628d2b0fd..42eb306b9dfc 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -239,14 +239,15 @@ def continue_to_exit(self, exitCode=0):
 
     def attach(self, program=None, pid=None, waitFor=None, trace=None,
                initCommands=None, preRunCommands=None, stopCommands=None,
-               exitCommands=None, attachCommands=None, coreFile=None):
+               exitCommands=None, attachCommands=None, coreFile=None, 
disconnectAutomatically=True):
         '''Build the default Makefile target, create the VSCode debug adaptor,
            and attach to the process.
         '''
         # Make sure we disconnect and terminate the VSCode debug adaptor even
         # if we throw an exception during the test case.
         def cleanup():
-            self.vscode.request_disconnect(terminateDebuggee=True)
+            if disconnectAutomatically:
+                self.vscode.request_disconnect(terminateDebuggee=True)
             self.vscode.terminate()
 
         # Execute the cleanup function during test case tear down.

diff  --git a/lldb/test/API/tools/lldb-vscode/disconnect/Makefile 
b/lldb/test/API/tools/lldb-vscode/disconnect/Makefile
new file mode 100644
index 000000000000..99998b20bcb0
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/disconnect/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py 
b/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
new file mode 100644
index 000000000000..2a4103a344a4
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
@@ -0,0 +1,82 @@
+"""
+Test lldb-vscode disconnect request
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import subprocess
+import time
+import os
+
+
+class TestVSCode_launch(lldbvscode_testcase.VSCodeTestCaseBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    source = 'main.cpp'
+
+    def disconnect_and_assert_no_output_printed(self):
+        self.vscode.request_disconnect()
+        # verify we didn't get any input after disconnect
+        time.sleep(2)
+        output = self.get_stdout()
+        self.assertTrue(output is None or len(output) == 0)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_launch(self):
+        """
+            This test launches a process that would creates a file, but we 
disconnect
+            before the file is created, which terminates the process and thus 
the file is not
+            created.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+
+        # We set a breakpoint right before the side effect file is created
+        self.set_source_breakpoints(self.source, [line_number(self.source, '// 
breakpoint')])
+        self.continue_to_next_stop()
+
+        self.vscode.request_disconnect()
+        # verify we didn't produce the side effect file
+        time.sleep(1)
+        self.assertFalse(os.path.exists(program + ".side_effect"))
+
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_attach(self):
+        """
+            This test attaches to a process that creates a file. We attach and 
disconnect
+            before the file is created, and as the process is not terminated 
upon disconnection,
+            the file is created anyway.
+        """
+        self.build_and_create_debug_adaptor()
+        program = self.getBuildArtifact("a.out")
+
+        # Use a file as a synchronization point between test and inferior.
+        sync_file_path = lldbutil.append_to_process_working_directory(self,
+            "sync_file_%d" % (int(time.time())))
+        self.addTearDownHook(
+            lambda: self.run_platform_command(
+                "rm %s" %
+                (sync_file_path)))
+
+        self.process = subprocess.Popen([program, sync_file_path])
+        lldbutil.wait_for_file_on_target(self, sync_file_path)
+
+        self.attach(pid=self.process.pid, disconnectAutomatically=False)
+        response = self.vscode.request_evaluate("wait_for_attach = false;")
+        self.assertTrue(response['success'])
+
+        # verify we haven't produced the side effect file yet
+        self.assertFalse(os.path.exists(program + ".side_effect"))
+
+        self.vscode.request_disconnect()
+        time.sleep(2)
+        # verify we produced the side effect file, as the program continued 
after disconnecting
+        self.assertTrue(os.path.exists(program + ".side_effect"))

diff  --git a/lldb/test/API/tools/lldb-vscode/disconnect/main.cpp 
b/lldb/test/API/tools/lldb-vscode/disconnect/main.cpp
new file mode 100644
index 000000000000..d3d7a4b7338a
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/disconnect/main.cpp
@@ -0,0 +1,33 @@
+#include <chrono>
+#include <cstdio>
+#include <fstream>
+#include <string>
+#include <thread>
+
+volatile bool wait_for_attach = true;
+
+void handle_attach(char *sync_file_path) {
+  lldb_enable_attach();
+
+  {
+    // Create a file to signal that this process has started up.
+    std::ofstream sync_file;
+    sync_file.open(sync_file_path);
+  }
+
+  while (wait_for_attach)
+    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+}
+
+int main(int argc, char **args) {
+  if (argc == 2)
+    handle_attach(args[1]);
+
+  // We let the binary live a little bit to see if it executed after detaching
+  // from // breakpoint
+
+  // Create a file to signal that this process has started up.
+  std::ofstream out_file; // breakpoint
+  out_file.open(std::string(args[0]) + ".side_effect");
+  return 0;
+}

diff  --git a/lldb/tools/lldb-vscode/VSCode.cpp 
b/lldb/tools/lldb-vscode/VSCode.cpp
index 36bc8ec8ebfd..81a8e10f24fe 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -38,7 +38,7 @@ VSCode::VSCode()
            {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
            {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
       focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
-      stop_at_entry(false) {
+      stop_at_entry(false), is_attach(false) {
   const char *log_file_path = getenv("LLDBVSCODE_LOG");
 #if defined(_WIN32)
 // Windows opens stdout and stdin in text mode which converts \n to 13,10

diff  --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h
index 5298d7554f4d..aceba485b68e 100644
--- a/lldb/tools/lldb-vscode/VSCode.h
+++ b/lldb/tools/lldb-vscode/VSCode.h
@@ -89,6 +89,7 @@ struct VSCode {
   lldb::tid_t focus_tid;
   bool sent_terminated_event;
   bool stop_at_entry;
+  bool is_attach;
   // Keep track of the last stop thread index IDs as threads won't go away
   // unless we send a "thread" event to indicate the thread exited.
   llvm::DenseSet<lldb::tid_t> thread_ids;

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index f1620d945fbc..3ec4c2c070af 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -514,6 +514,7 @@ void SetSourceMapFromArguments(const llvm::json::Object 
&arguments) {
 //   }]
 // }
 void request_attach(const llvm::json::Object &request) {
+  g_vsc.is_attach = true;
   llvm::json::Object response;
   lldb::SBError error;
   FillResponse(request, response);
@@ -769,7 +770,9 @@ void request_disconnect(const llvm::json::Object &request) {
   FillResponse(request, response);
   auto arguments = request.getObject("arguments");
 
-  bool terminateDebuggee = GetBoolean(arguments, "terminateDebuggee", false);
+  bool defaultTerminateDebuggee = g_vsc.is_attach ? false : true;
+  bool terminateDebuggee =
+      GetBoolean(arguments, "terminateDebuggee", defaultTerminateDebuggee);
   lldb::SBProcess process = g_vsc.target.GetProcess();
   auto state = process.GetState();
 
@@ -788,10 +791,9 @@ void request_disconnect(const llvm::json::Object &request) 
{
   case lldb::eStateStopped:
   case lldb::eStateRunning:
     g_vsc.debugger.SetAsync(false);
-    if (terminateDebuggee)
-      process.Kill();
-    else
-      process.Detach();
+    lldb::SBError error = terminateDebuggee ? process.Kill() : 
process.Detach();
+    if (!error.Success())
+      response.try_emplace("error", error.GetCString());
     g_vsc.debugger.SetAsync(true);
     break;
   }
@@ -1357,6 +1359,7 @@ void request_initialize(const llvm::json::Object 
&request) {
 //   }]
 // }
 void request_launch(const llvm::json::Object &request) {
+  g_vsc.is_attach = false;
   llvm::json::Object response;
   lldb::SBError error;
   FillResponse(request, response);


        
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to