[Lldb-commits] [lldb] 63e5121 - Fix race condition when launching and attaching.

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Thu Mar 3 16:20:36 PST 2022


Author: Greg Clayton
Date: 2022-03-03T16:20:27-08:00
New Revision: 63e512100ac62efd5f009c6b490194e973a9d1ce

URL: https://github.com/llvm/llvm-project/commit/63e512100ac62efd5f009c6b490194e973a9d1ce
DIFF: https://github.com/llvm/llvm-project/commit/63e512100ac62efd5f009c6b490194e973a9d1ce.diff

LOG: Fix race condition when launching and attaching.

This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797
This version only waits for the process to stop when using "launchCommands" or "attachCommands"...

...and doesn't play with the async mode when doing normal launch/attach.

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 "launchCommands" or "attachCommands" are run to ensure that we have a stopped process point that is ready for the debug session to proceed. We spin up the thread that listens for process events before we start the launch or attach, but we don't want stop events being delivered through the DAP protocol until the "configurationDone" packet is received. We now always ignore the stop event with a stop ID of 1, which is the first stop. All normal launch and attach scenarios use the synchronous mode, and "launchCommands and "attachCommands" run an array of LLDB commands in async mode.

This should make our launch with "launchCommands" and attach with "attachCommands" avoid a race condition when the process is being launched or attached.

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

Added: 
    

Modified: 
    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
    lldb/tools/lldb-vscode/package.json

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index ff798364c9573..8c0000bdb1546 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -374,7 +374,7 @@ def test_commands(self):
     @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")

diff  --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp
index 8e43ea3b771e6..adc4549f7c25d 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -39,8 +39,8 @@ 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), 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 +528,46 @@ void VSCode::RegisterRequestCallback(std::string request,
   request_handlers[request] = callback;
 }
 
+lldb::SBError VSCode::WaitForProcessToStop(uint32_t seconds) {
+  lldb::SBError error;
+  lldb::SBProcess process = target.GetProcess();
+  if (!process.IsValid()) {
+    error.SetErrorString("invalid process");
+    return error;
+  }
+  auto timeout_time =
+      std::chrono::steady_clock::now() + std::chrono::seconds(seconds);
+  while (std::chrono::steady_clock::now() < timeout_time) {
+    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 during launch or attach");
+        return error;
+      case lldb::eStateExited:
+        error.SetErrorString("process exited during launch or attach");
+        return error;
+      case lldb::eStateUnloaded:
+        error.SetErrorString("process unloaded during launch or attach");
+        return error;
+      case lldb::eStateCrashed:
+      case lldb::eStateStopped:
+        return lldb::SBError(); // Success!
+    }
+    std::this_thread::sleep_for(std::chrono::microseconds(250));
+  }
+  error.SetErrorStringWithFormat("process failed to stop within %u seconds",
+                                 seconds);
+  return error;
+}
+
 void Variables::Clear() {
   locals.Clear();
   globals.Clear();

diff  --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h
index 602cf758a9a17..aa325038b2e79 100644
--- a/lldb/tools/lldb-vscode/VSCode.h
+++ b/lldb/tools/lldb-vscode/VSCode.h
@@ -145,6 +145,7 @@ struct VSCode {
   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,27 @@ struct VSCode {
   /// Debuggee will continue from stopped state.
   void WillContinue() { variables.Clear(); }
 
+  /// Poll the process to wait for it to reach the eStateStopped state.
+  ///
+  /// Wait for the process hit a stopped state. When running a launch with
+  /// "launchCommands", or attach with  "attachCommands", the calls might take
+  /// some time to stop at the entry point since the command is asynchronous. 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. We must use
+  /// polling because "attachCommands" or "launchCommands" may or may not send
+  /// process state change events depending on if the user modifies the async
+  /// setting in the debugger. Since both "attachCommands" and "launchCommands"
+  /// could end up using any combination of LLDB commands, we must ensure we can
+  /// also catch when the process stops, so we must poll the process to make
+  /// sure we handle all cases.
+  ///
+  /// \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

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 97ec4b578cf7c..28c0dfbac40f8 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -449,10 +449,18 @@ void EventThreadFunction() {
           case lldb::eStateSuspended:
             break;
           case lldb::eStateStopped:
-            // Only report a stopped event if the process was not restarted.
-            if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
-              SendStdOutStdErr(process);
-              SendThreadStoppedEvent();
+            // We launch and attach in synchronous mode then the first stop
+            // event will not be delivered. If we use "launchCommands" during a
+            // launch or "attachCommands" during an attach we might some process
+            // stop events which we do not want to send an event for. We will
+            // manually send a stopped event in request_configurationDone(...)
+            // so don't send any before then.
+            if (g_vsc.configuration_done_sent) {
+              // Only report a stopped event if the process was not restarted.
+              if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
+                SendStdOutStdErr(process);
+                SendThreadStoppedEvent();
+              }
             }
             break;
           case lldb::eStateRunning:
@@ -600,6 +608,7 @@ void request_attach(const llvm::json::Object &request) {
   g_vsc.terminate_commands = GetStrings(arguments, "terminateCommands");
   auto attachCommands = GetStrings(arguments, "attachCommands");
   llvm::StringRef core_file = GetString(arguments, "coreFile");
+  const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
   g_vsc.stop_at_entry =
       core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
   std::vector<std::string> postRunCommands =
@@ -657,6 +666,10 @@ void request_attach(const llvm::json::Object &request) {
     // 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 attached and stopped before proceeding as the
+    // the launch commands are not run using the synchronous mode.
+    error = g_vsc.WaitForProcessToStop(timeout_seconds);
   }
 
   if (error.Success() && core_file.empty()) {
@@ -787,6 +800,7 @@ void request_configurationDone(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+  g_vsc.configuration_done_sent = true;
   if (g_vsc.stop_at_entry)
     SendThreadStoppedEvent();
   else
@@ -1652,6 +1666,7 @@ void request_launch(const llvm::json::Object &request) {
       GetStrings(arguments, "postRunCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
+  const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
@@ -1726,6 +1741,10 @@ void request_launch(const llvm::json::Object &request) {
     // 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 as the the launch commands are not run using the synchronous
+    // mode.
+    error = g_vsc.WaitForProcessToStop(timeout_seconds);
   }
 
   if (error.Fail()) {

diff  --git a/lldb/tools/lldb-vscode/package.json b/lldb/tools/lldb-vscode/package.json
index a5c79911f6e9f..c5d7f2445619d 100644
--- a/lldb/tools/lldb-vscode/package.json
+++ b/lldb/tools/lldb-vscode/package.json
@@ -215,7 +215,7 @@
 							},
 							"launchCommands": {
 								"type": "array",
-								"description": "Custom commands that are executed instead of launching a process. A target will be created with the launch arguments prior to executing these commands. The commands may optionally create a new target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail.",
+								"description": "Custom commands that are executed instead of launching a process. A target will be created with the launch arguments prior to executing these commands. The commands may optionally create a new target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail. Launch the process with \"process launch -s\" to make the process to at the entry point since lldb-vscode will auto resume if necessary.",
 								"default": []
 							},
 							"stopCommands": {
@@ -232,6 +232,10 @@
 								"type": "boolean",
 								"description": "Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs",
 								"default": false
+							},
+							"timeout": {
+								"type": "string",
+								"description": "The time in seconds to wait for a program to stop at entry point when launching with \"launchCommands\". Defaults to 30 seconds."
 							}
 						}
 					},
@@ -307,6 +311,10 @@
 							"coreFile": {
 								"type": "string",
 								"description": "Path to the core file to debug."
+							},
+							"timeout": {
+								"type": "string",
+								"description": "The time in seconds to wait for a program to stop when attaching using \"attachCommands\". Defaults to 30 seconds."
 							}
 						}
 					}


        


More information about the lldb-commits mailing list