[Lldb-commits] [lldb] a59014b - Revert "Fix race condition when launching and attaching."

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Fri Feb 18 03:11:47 PST 2022


Author: Pavel Labath
Date: 2022-02-18T12:11:37+01:00
New Revision: a59014b759050af93e0ab214dcbf0cc2dd75bb75

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

LOG: Revert "Fix race condition when launching and attaching."

It breaks TestVSCode_attach.py.

This reverts commit 9febd1e573fb8b3d1de5844b7bfd33eb998f0106 and
38054556a08884aa15d3ebc720e2f43d0cb5a944.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index faa0b93b3f9a7..603b1545cd714 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -228,9 +228,6 @@ def handle_recv_packet(self, packet):
                 # '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

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 8c0000bdb1546..ff798364c9573 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 "launchCommands" with extra launching settings
+            Tests the "luanchCommands" 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 a6fe7f840a566..3209eea4a897f 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -528,58 +528,6 @@ void VSCode::RegisterRequestCallback(std::string request,
   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.
-  // We must use polling because attach doesn't send a process state change
-  // event for the first stop, while launching does. 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.
-
-  lldb::SBError error;
-  lldb::SBProcess process = target.GetProcess();
-  if (!process.IsValid()) {
-    error.SetErrorString("invalid process");
-    return error;
-  }
-  auto timeout_time =
-      std::chrono::high_resolution_clock::now() + std::chrono::seconds(seconds);
-  while (std::chrono::high_resolution_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 bc868760eb830..602cf758a9a17 100644
--- a/lldb/tools/lldb-vscode/VSCode.h
+++ b/lldb/tools/lldb-vscode/VSCode.h
@@ -243,19 +243,6 @@ struct VSCode {
   /// Debuggee will continue from stopped state.
   void WillContinue() { variables.Clear(); }
 
-  /// Poll the process to wait for it to reach the eStateStopped state.
-  ///
-  /// We need to ensure the process is stopped and ready to resume before we
-  /// continue with the launch or attach. This is needed since we no longer play
-  /// with the synchronous mode in the debugger for launching (with or without
-  /// "launchCommands") or attaching (with or without "attachCommands").
-  ///
-  /// \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 734b23afc9b28..97ec4b578cf7c 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -449,18 +449,10 @@ void EventThreadFunction() {
           case lldb::eStateSuspended:
             break;
           case lldb::eStateStopped:
-            // Now that we don't mess with the async setting in the debugger
-            // when launching or attaching we will get the first process stop
-            // event which we do not want to send an event for. This is because
-            // we either manually deliver the event in by calling the
-            // SendThreadStoppedEvent() from request_configuarationDone() if we
-            // want to stop on entry, or we resume from that function.
-            if (process.GetStopID() > 1) {
-              // Only report a stopped event if the process was not restarted.
-              if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
-                SendStdOutStdErr(process);
-                SendThreadStoppedEvent();
-              }
+            // Only report a stopped event if the process was not restarted.
+            if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
+              SendStdOutStdErr(process);
+              SendThreadStoppedEvent();
             }
             break;
           case lldb::eStateRunning:
@@ -608,7 +600,6 @@ 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 =
@@ -649,10 +640,15 @@ void request_attach(const llvm::json::Object &request) {
   }
   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
@@ -662,9 +658,6 @@ void request_attach(const llvm::json::Object &request) {
     // selected target after these commands are run.
     g_vsc.target = g_vsc.debugger.GetSelectedTarget();
   }
-  // Make sure the process is attached and stopped before proceeding.
-  if (error.Success())
-    error = g_vsc.WaitForProcessToStop(timeout_seconds);
 
   if (error.Success() && core_file.empty()) {
     auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
@@ -1659,7 +1652,6 @@ 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
@@ -1724,17 +1716,17 @@ void request_launch(const llvm::json::Object &request) {
     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(timeout_seconds);
 
   if (error.Fail()) {
     response["success"] = llvm::json::Value(false);

diff  --git a/lldb/tools/lldb-vscode/package.json b/lldb/tools/lldb-vscode/package.json
index bedc8f16ea26e..a5c79911f6e9f 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. Launch the process with \"process launch -s\" to make the process to at the entry point since lldb-vscode will auto resume if necessary.",
+								"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.",
 								"default": []
 							},
 							"stopCommands": {
@@ -232,10 +232,6 @@
 								"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. Defaults to 30 seconds."
 							}
 						}
 					},
@@ -311,10 +307,6 @@
 							"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. Defaults to 30 seconds."
 							}
 						}
 					}


        


More information about the lldb-commits mailing list