clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, jingham, aadsm, wallace, 
ilya-nozhkin.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We discovered that when using "launchCommands" or "attachCommands" that there 
was an issue where these commands were not being run synchronously. There were 
further problems in this case where we would get thread events for the process 
that was just launched or attached before the IDE was ready, which is after 
"configurationDone" was sent to lldb-vscode.

This fix introduces the ability to wait for the process to stop after the run 
or attach to ensure that we have a stopped process at the entry point that is 
ready for the debug session to proceed. This also allows us to run the normal 
launch or attach without needing to play with the async flag the debugger. We 
spin up the thread that listens for process events before we start the launch 
or attach, but we stop events from being delivered through the DAP protocol 
until the "configurationDone" packet is received. This function was manually 
delivering the first process stopped event in the code already, so this means 
we don't end up sending two stopped events at the start of a debug session.

Added a flag to the vscode.py protocol classes that detects and ensures that no 
"stopped" events are sent prior to "configurationDone" has been sent and will 
raise an error if it does happen.

This should make our launching and attaching more reliable and avoid some 
deadlocks that were being seen (https://reviews.llvm.org/D119548).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119797

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -449,6 +449,11 @@
           case lldb::eStateSuspended:
             break;
           case lldb::eStateStopped:
+            // We don't want any proces events before configurationDone sets
+            // value to true where the request_configurationDone() function will
+            // manually call SendThreadStoppedEvent().
+            if (!g_vsc.configuration_done_sent)
+              break;
             // Only report a stopped event if the process was not restarted.
             if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
               SendStdOutStdErr(process);
@@ -640,15 +645,10 @@
   }
   if (attachCommands.empty()) {
     // No "attachCommands", just attach normally.
-    // Disable async events so the attach will be successful when we return from
-    // the launch call and the launch will happen synchronously
-    g_vsc.debugger.SetAsync(false);
     if (core_file.empty())
       g_vsc.target.Attach(attach_info, error);
     else
       g_vsc.target.LoadCore(core_file.data(), error);
-    // Reenable async events
-    g_vsc.debugger.SetAsync(true);
   } else {
     // We have "attachCommands" that are a set of commands that are expected
     // to execute the commands after which a process should be created. If there
@@ -658,6 +658,8 @@
     // selected target after these commands are run.
     g_vsc.target = g_vsc.debugger.GetSelectedTarget();
   }
+  if (error.Success())
+    error = g_vsc.WaitForProcessToStop(/*seconds=*/10);
 
   if (error.Success() && core_file.empty()) {
     auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
@@ -787,7 +789,12 @@
   llvm::json::Object response;
   FillResponse(request, response);
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+  // We are already stopped so now we are ready to listen for process events
+  // and forward them to the DAP as events.
+  g_vsc.configuration_done_sent = true;
   if (g_vsc.stop_at_entry)
+    // We manually send a thread stopped event here since the standard launch
+    // or attach API calls will eat the stopped event. But "launchCommands"
     SendThreadStoppedEvent();
   else
     g_vsc.target.GetProcess().Continue();
@@ -1716,17 +1723,17 @@
     if (llvm::Error err = request_runInTerminal(request))
       error.SetErrorString(llvm::toString(std::move(err)).c_str());
   } else if (launchCommands.empty()) {
-    // Disable async events so the launch will be successful when we return from
-    // the launch call and the launch will happen synchronously
-    g_vsc.debugger.SetAsync(false);
     g_vsc.target.Launch(launch_info, error);
-    g_vsc.debugger.SetAsync(true);
   } else {
     g_vsc.RunLLDBCommands("Running launchCommands:", launchCommands);
     // The custom commands might have created a new target so we should use the
     // selected target after these commands are run.
     g_vsc.target = g_vsc.debugger.GetSelectedTarget();
   }
+  // Make sure the process is launched and stopped at the entry point before
+  // proceeding.
+  if (error.Success())
+    error = g_vsc.WaitForProcessToStop(/*seconds=*/10);
 
   if (error.Fail()) {
     response["success"] = llvm::json::Value(false);
Index: lldb/tools/lldb-vscode/VSCode.h
===================================================================
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -145,6 +145,7 @@
   bool sent_terminated_event;
   bool stop_at_entry;
   bool is_attach;
+  bool configuration_done_sent;
   uint32_t reverse_request_seq;
   std::map<std::string, RequestCallback> request_handlers;
   bool waiting_for_run_in_terminal;
@@ -243,6 +244,15 @@
   /// Debuggee will continue from stopped state.
   void WillContinue() { variables.Clear(); }
 
+  /// Poll the process to wait for eStateStopped. This is needed when we use
+  /// "launchCommands" or "attachCommands" to launch a process.
+  ///
+  /// \param[in] seconds
+  ///   The number of seconds to poll the process to wait until it is stopped.
+  ///
+  /// \return Error if waiting for the process fails, no error if succeeds.
+  lldb::SBError WaitForProcessToStop(uint32_t seconds);
+
 private:
   // Send the JSON in "json_str" to the "out" stream. Correctly send the
   // "Content-Length:" field followed by the length, followed by the raw
Index: lldb/tools/lldb-vscode/VSCode.cpp
===================================================================
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <unistd.h>
 #include <chrono>
 #include <cstdarg>
 #include <fstream>
@@ -39,8 +40,8 @@
            {"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), is_attach(false), reverse_request_seq(0),
-      waiting_for_run_in_terminal(false),
+      stop_at_entry(false), is_attach(false), configuration_done_sent(false),
+      reverse_request_seq(0), waiting_for_run_in_terminal(false),
       progress_event_reporter(
           [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }) {
   const char *log_file_path = getenv("LLDBVSCODE_LOG");
@@ -528,6 +529,52 @@
   request_handlers[request] = callback;
 }
 
+lldb::SBError VSCode::WaitForProcessToStop(uint32_t seconds) {
+  // Wait for the process hit a stopped state. When running a launch (with or
+  // without "launchCommands") or attach (with or without)= "attachCommands"),
+  // the calls might take some time to stop at the entry point since the command
+  // is asynchronous. So we need to sync up with the process and make sure it is
+  // stopped before we proceed to do anything else as we will soon be asked to
+  // set breakpoints and other things that require the process to be stopped.
+  lldb::SBError error;
+  lldb::SBProcess process = target.GetProcess();
+  if (!process.IsValid()) {
+    error.SetErrorString("invalid process");
+    return error;
+  }
+  const useconds_t usleep_interval = 250 * 10000; // 250 ms internal
+  const useconds_t count = (seconds * 1000 * 1000)/usleep_interval;
+  for (useconds_t i=0; i<count; i++) {
+    const auto state = process.GetState();
+    switch (state) {
+      case lldb::eStateAttaching:
+      case lldb::eStateConnected:
+      case lldb::eStateInvalid:
+      case lldb::eStateLaunching:
+      case lldb::eStateRunning:
+      case lldb::eStateStepping:
+      case lldb::eStateSuspended:
+        break;
+      case lldb::eStateDetached:
+        error.SetErrorString("process detached after running 'launchCommands'");
+        return error;
+      case lldb::eStateExited:
+        error.SetErrorString("process exited after running 'launchCommands'");
+        return error;
+      case lldb::eStateUnloaded:
+        error.SetErrorString("process unloaded after running 'launchCommands'");
+        return error;
+      case lldb::eStateCrashed:
+      case lldb::eStateStopped:
+        return lldb::SBError(); // Success!
+    }
+    usleep(usleep_interval);
+  }
+  error.SetErrorStringWithFormat("process failed to stop within %u seconds",
+                                 seconds);
+  return error;
+}
+
 void Variables::Clear() {
   locals.Clear();
   globals.Clear();
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -374,7 +374,7 @@
     @skipIfRemote
     def test_extra_launch_commands(self):
         '''
-            Tests the "luanchCommands" with extra launching settings
+            Tests the "launchCommands" with extra launching settings
         '''
         self.build_and_create_debug_adaptor()
         program = self.getBuildArtifact("a.out")
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -228,6 +228,9 @@
                 # 'stopped' event. We need to remember the thread stop
                 # reasons since the 'threads' command doesn't return
                 # that information.
+                if not self.configuration_done_sent:
+                    raise ValueError("'stopped' event received before "
+                                     "configuationDone packet was sent")
                 self._process_stopped()
                 tid = body['threadId']
                 self.thread_stop_reasons[tid] = body
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to