[lldb-vscode] Support processId returned by the IDE in the runInTerminal request

[wallace]

Summary

Currently, the runInTerminal is implemented by using --wait-for underneath, which is error prone as it often can't attach on Linux. A reason I've found is that LLDB does process polling by using commands like "ps", which are not standard across distributions. This is a problem that doesn't happen on Mac.

For more context, the debug adapter protocol specifies that the response of the "runInTerminal" reverse request doesn't need to provide the pid of the process launched by the IDE. VSCode, for example, doesn't return the pid. Without this pid, the only way to attach to the process is with --wait-for.

However, IDEs like VSCode support customizations of their debuggers, allowing it to return the pid. In this case, lldb-vscode needs to know that the IDE will return the pid and therefore it shouldn't use --wait-for. Because of this, lldb-vscode will also require the launch request to include 'runInTerminalRespondsWithPid: true' to work this way. This is acceptable, as the user/developer is already customizing the IDE, so doings the pid addition and the runInTerminalRespondsWithPid inclusion are simple changes.

This new approach to runInTerminal is guaranteed to attach to the process, as long as it doesn't finish right away, which is better than what we had before.

Tested on Mac and Linux

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

class VSCodeTestCaseBase(TestBase):
     def continue_to_next_stop(self):
         self.vscode.request_continue()
         return self.vscode.wait_for_stopped()
 
     def continue_to_breakpoints(self, breakpoint_ids):
         self.vscode.request_continue()
         self.verify_breakpoint_hit(breakpoint_ids)
 
+    '''
+        Continues successive stop points until any of the provided breakpoints is hit.
+        If an unwanted breakpoint is hit after the given timeout, this method throws an
+        exception.
+    '''
+    def continue_until_any_of_breakpoints_is_hit(self, breakpoint_ids, timeout_in_sec=10):
+        t0 = time.time()
+        while (True):
+            self.vscode.request_continue()
+            try:
+                self.verify_breakpoint_hit(breakpoint_ids)
+                break
+            except:
+                t = time.time() - t0
+                if t > timeout_in_sec:
+                    raise
+
     def continue_to_exception_breakpoint(self, filter_label):
         self.vscode.request_continue()
         self.assertTrue(self.verify_exception_breakpoint_hit(filter_label),
                         'verify we got "%s"' % (filter_label))
 
     def continue_to_exit(self, exitCode=0):
         self.vscode.request_continue()
         stopped_events = self.vscode.wait_for_stopped()
def attach(self, program=None, pid=None, waitFor=None, trace=None,
                             'attach failed (%s)' % (response['message']))
 
     def launch(self, program=None, args=None, cwd=None, env=None,
                stopOnEntry=False, disableASLR=True,
                disableSTDIO=False, shellExpandArguments=False,
                trace=False, initCommands=None, preRunCommands=None,
                stopCommands=None, exitCommands=None, terminateCommands=None,
                sourcePath=None, debuggerRoot=None, launchCommands=None,
-               sourceMap=None, disconnectAutomatically=True, runInTerminal=False):
+               sourceMap=None, disconnectAutomatically=True, runInTerminal=False,
+               runInTerminalRespondsWithPid=False):
         '''Sending launch request to vscode
         '''
 
         # Make sure we disconnect and terminate the VSCode debug adapter,
         # if we throw an exception during the test case
         def cleanup():
             if disconnectAutomatically:
                 self.vscode.request_disconnect(terminateDebuggee=True)
def launch(self, program=None, args=None, cwd=None, env=None,
             preRunCommands=preRunCommands,
             stopCommands=stopCommands,
             exitCommands=exitCommands,
             terminateCommands=terminateCommands,
             sourcePath=sourcePath,
             debuggerRoot=debuggerRoot,
             launchCommands=launchCommands,
             sourceMap=sourceMap,
-            runInTerminal=runInTerminal)
+            runInTerminal=runInTerminal,
+            runInTerminalRespondsWithPid=runInTerminalRespondsWithPid)
         if not (response and response['success']):
             self.assertTrue(response['success'],
                             'launch failed (%s)' % (response['message']))
         # We need to trigger a request_configurationDone after we've successfully
         # attached a runInTerminal process to finish initialization.
         if runInTerminal:
             self.vscode.request_configurationDone()
 
 
     def build_and_launch(self, program, args=None, cwd=None, env=None,
                          stopOnEntry=False, disableASLR=True,
                          disableSTDIO=False, shellExpandArguments=False,
                          trace=False, initCommands=None, preRunCommands=None,
                          stopCommands=None, exitCommands=None,
                          terminateCommands=None, sourcePath=None,
-                         debuggerRoot=None, runInTerminal=False):
+                         debuggerRoot=None, runInTerminal=False,
+                         runInTerminalRespondsWithPid=False):
         '''Build the default Makefile target, create the VSCode debug adaptor,
            and launch the process.
         '''
         self.build_and_create_debug_adaptor()
         self.assertTrue(os.path.exists(program), 'executable must exist')
 
         self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
                     disableSTDIO, shellExpandArguments, trace,
                     initCommands, preRunCommands, stopCommands, exitCommands,
-                    terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal)
+                    terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal,
+                    runInTerminalRespondsWithPid=runInTerminalRespondsWithPid)

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

def send_recv(self, command):
             if response_or_request is None:
                 desc = 'no response for "%s"' % (command['command'])
                 raise ValueError(desc)
             if response_or_request['type'] == 'response':
                 self.validate_response(command, response_or_request)
                 return response_or_request
             else:
                 if response_or_request['command'] == 'runInTerminal':
-                    subprocess.Popen(response_or_request['arguments']['args'], 
+                    inferior = subprocess.Popen(response_or_request['arguments']['args'],
                         env=response_or_request['arguments']['env'])
                     self.send_packet({
                         "type": "response",
                         "seq": -1,
                         "request_seq": response_or_request['seq'],
                         "success": True,
                         "command": "runInTerminal",
-                        "body": {}
+                        "body": {
+                            "processId": inferior.pid
+                        }
                     }, set_sequence=False)
                 else:
                     desc = 'unkonwn reverse request "%s"' % (response_or_request['command'])
                     raise ValueError(desc)
             
         return None
 
     def wait_for_event(self, filter=None, timeout=None):
class DebugCommunication(object):
 
     def request_launch(self, program, args=None, cwd=None, env=None,
                        stopOnEntry=False, disableASLR=True,
                        disableSTDIO=False, shellExpandArguments=False,
                        trace=False, initCommands=None, preRunCommands=None,
                        stopCommands=None, exitCommands=None,
                        terminateCommands=None ,sourcePath=None,
                        debuggerRoot=None, launchCommands=None, sourceMap=None,
-                       runInTerminal=False):
+                       runInTerminal=False, runInTerminalRespondsWithPid=False):
         args_dict = {
             'program': program
         }
         if args:
             args_dict['args'] = args
         if cwd:
             args_dict['cwd'] = cwd
         if env:
def request_launch(self, program, args=None, cwd=None, env=None,
         if debuggerRoot:
             args_dict['debuggerRoot'] = debuggerRoot
         if launchCommands:
             args_dict['launchCommands'] = launchCommands
         if sourceMap:
             args_dict['sourceMap'] = sourceMap
         if runInTerminal:
             args_dict['runInTerminal'] = runInTerminal
+            args_dict['runInTerminalRespondsWithPid'] = runInTerminalRespondsWithPid
         command_dict = {
             'command': 'launch',
             'type': 'request',
             'arguments': args_dict
         }
         response = self.send_recv(command_dict)
 
         # Wait for a 'process' and 'initialized' event in any order

lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py

def test_runInTerminal(self):
         self.assertEqual(argc, 2)
 
         argv1 = self.vscode.request_evaluate('argv[1]')['body']['result']
         self.assertIn('foobar', argv1)
 
         # We verify we were able to set the environment
         env = self.vscode.request_evaluate('foo')['body']['result']
         self.assertIn('bar', env)
+
+    @skipIfRemote
+    def test_runInTerminalWithPidProvidedByClient(self):
+        '''
+            Tests the "runInTerminal" reverse request when the client provides
+            the inferior's pid in the "runInTerminal" response.
+        '''
+        program = self.getBuildArtifact("a.out")
+        source = 'main.c'
+        self.build_and_launch(program, stopOnEntry=True, runInTerminal=True, args=["foobar"], env=["FOO=bar"],
+            runInTerminalRespondsWithPid=True)
+        breakpoint_line = line_number(source, '// breakpoint')
+
+        bpts = self.set_source_breakpoints(source, [breakpoint_line])
+        # We have to do this instead of continue_to_next_stop because
+        # sometimes lldb stops for a second time inside the sleep method.
+        # That's something that needs to be fixed in a different patch.
+        self.continue_until_any_of_breakpoints_is_hit(bpts)
+
+        # We verify we actually stopped inside the loop
+        counter = int(self.vscode.get_local_variable_value('counter'))
+        self.assertTrue(counter > 0)
+
+        # We verify we were able to set the launch arguments
+        argc = int(self.vscode.get_local_variable_value('argc'))
+        self.assertEqual(argc, 2)
+
+        argv1 = self.vscode.request_evaluate('argv[1]')['body']['result']
+        self.assertIn('foobar', argv1)
+
+        # We verify we were able to set the environment
+        env = self.vscode.request_evaluate('foo')['body']['result']
+        self.assertIn('bar', env)

lldb/tools/lldb-vscode/lldb-vscode.cpp

 void request_runInTerminal(const llvm::json::Object &launch_request,
                            llvm::json::Object &launch_response) {
   // We have already created a target that has a valid "program" path to the
   // executable. We will attach to the next process whose name matches that
   // of the target's.
   g_vsc.is_attach = true;
   lldb::SBAttachInfo attach_info;
   lldb::SBError error;
+  const llvm::json::Object *arguments = launch_request.getObject("arguments");
+  bool runInTerminalRespondsWithPid =
+      GetBoolean(*arguments, "runInTerminalRespondsWithPid", false);
+
+  // If the client IDE won't provide the pid of the debuggee, then we attach
+  // with "wait for"
+  if (!runInTerminalRespondsWithPid) {
     attach_info.SetWaitForLaunch(true, /*async*/ true);
     g_vsc.target.Attach(attach_info, error);
+  }
 
   llvm::json::Object reverse_request =
       CreateRunInTerminalReverseRequest(launch_request);
   llvm::json::Object reverse_response;
   lldb_vscode::PacketStatus status =
       g_vsc.SendReverseRequest(reverse_request, reverse_response);
   if (status != lldb_vscode::PacketStatus::Success)
     error.SetErrorString("Process cannot be launched by IDE.");
 
   if (error.Success()) {
+    if (runInTerminalRespondsWithPid) {
+      lldb::pid_t pid =
+          GetUnsigned(reverse_response, "processId", LLDB_INVALID_PROCESS_ID);
+      attach_info.SetProcessID(pid);
+      g_vsc.debugger.SetAsync(false);
+      g_vsc.target.Attach(attach_info, error);
+      g_vsc.debugger.SetAsync(true);
+    } else {
       // Wait for the attach stop event to happen or for a timeout.
       g_vsc.waiting_for_run_in_terminal = true;
       static std::mutex mutex;
       std::unique_lock<std::mutex> locker(mutex);
       g_vsc.request_in_terminal_cv.wait_for(locker, std::chrono::seconds(10));
+    }
+  }
 
-    auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
-    if (attached_pid == LLDB_INVALID_PROCESS_ID)
-      error.SetErrorString("Failed to attach to a process");
-    else
-      SendProcessEvent(Attach);
-  }
 
   if (error.Fail()) {
     launch_response["success"] = llvm::json::Value(false);
     EmplaceSafeString(launch_response, "message",
                       std::string(error.GetCString()));
   } else {
     launch_response["success"] = llvm::json::Value(true);
     g_vsc.SendJSON(CreateEventObject("initialized"));

lldb/tools/lldb-vscode/package.json

"debuggers": [
               },
               "exitCommands": {
                 "type": "array",
                 "description": "Commands executed at the end of debugging session.",
                 "default": []
               },
               "runInTerminal": {
                 "type": "boolean",
-                "description": "Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs",
+                "description": "Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs. If the client IDE returns the \"processId\" as part of the \"runInTerminal\" reverse request, then it's suggested that the client IDE includes the field '\"runInTerminalRespondsWithPid\": true' as part of the \"launch\" request, as LLDB can guarantee attachment to the process this way.",
                 "default": false
               }
             }
           },
           "attach": {
             "properties": {
               "program": {
                 "type": "string",