Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only waits for the process to stop when using "launchCommands" or "attachCommands"...

[clayborg]

Summary

...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.

[wallace]

I'm assuming this passes all tests locally

[labath]

I'm not exactly exactly thrilled by the sleep-based implementation, but otherwise, the patch seems fine.

lldb/tools/lldb-vscode/VSCode.cpp

551

steady_clock would be better here (no time travel there)

[yinghuitan]

lldb/tools/lldb-vscode/VSCode.cpp

532–533

The same: update the comment to reflect latest state.

551

Agreed.

lldb/tools/lldb-vscode/VSCode.h

250–252

This comment is not true in this diff anymore. We only use async mode in launchCommands/attachCommands.

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

459

This assumes there is only one stop event sent before request_configuarationDone. This is true in our detected situation but I like the other logic of not sending any stop event before request_configuarationDone. That is more accurately aligned with synchronous launch/attach mode withotu relying on one stop event assumption for some future unknown situation.

[clayborg]

Fixes:

• use a "configuration_done_sent" boolean to track when the configurationDone packet has already been sent and stop all process events from being sent before this happens. This allows "launchCommands" and "attachCommands" to have more than one stop if desired before we sync up and make sure the process is stopped after running these commands
• fixed comments to reflect the fact that we are only synchronizing with the process if "attachCommands" or "launchCommands" are used
• use steady clock

[labath]

Greg, this is still causing the TestVSCode_attach.py test to fail on lldb-x86_64-debian (it would probably fail elsewhere as well, but the test is already disabled everywhere else). Judging by the error message (attach failed (process exited during launch or attach)), it definitely seems related to this patch. Can you do something about it?

[clayborg]

In D120755#3364802, @labath wrote:

Greg, this is still causing the TestVSCode_attach.py test to fail on lldb-x86_64-debian (it would probably fail elsewhere as well, but the test is already disabled everywhere else). Judging by the error message (attach failed (process exited during launch or attach)), it definitely seems related to this patch. Can you do something about it?

I will check into this, yes

[clayborg]

I think I found the root cause of the issue in linux and it might fix this test for many other platforms:

commit c41c57468949036a06695f06a97a8e14e8b252bd (HEAD -> main)
Author: Greg Clayton <[e-mail]>
Date: Mon Mar 7 11:31:21 2022 -0800

Fix buildbots after https://reviews.llvm.org/D120755.

This improves this test a lot because before when using the "attachCommands" to run the following commands:

(lldb) target create -d /path/to/a.out
(lldb) process launch

This was racy as it wasn't stopping the program at the entry point, and the process might run to completion before we can even debug it. With the recent changes to the "attachCommands" we were waiting for the process to stop, but the process might be exited already, and that _should_ have caused the attach to fail since there was no process to attach to. By adding "--stop-at-entry" to the process launch, we ensure this should be less racy and give us a valid process to attach to.

[clayborg]

https://reviews.llvm.org/rGc41c57468949

[clayborg]

The problem seemed to be we had a test case that was kind of bogus that was running attach commands with:

"target create ..." followed by "process launch". There was no synchronization before, but now that is. The problem is that the "process launch" was run, but the process could run to the exit stage before we synchronized with it. So it was correctly saying that the attach failed, because we weren't able to actually attach to the process as it had run to the exit point by the time we tried to attach. Fix this by adding "--stop-at-entry" to the "process launch" command which will ensure it does stop and it validly attached.

[clayborg]

I verified that my linux server was failing the attach in the same way and it is now fixed with the Buildbot patch

lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

def test_commands(self):
         output = self.collect_console(duration=1.0)
         self.verify_commands('exitCommands', output, exitCommands)
         self.verify_commands('terminateCommands', output, terminateCommands)
 
     @skipIfWindows
     @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")
 
         source = 'main.c'
         first_line = line_number(source, '// breakpoint 1')
         second_line = line_number(source, '// breakpoint 2')
         # Set target binary and 2 breakpoints

lldb/tools/lldb-vscode/VSCode.h

struct VSCode {
   std::vector<std::string> pre_run_commands;
   std::vector<std::string> exit_commands;
   std::vector<std::string> stop_commands;
   std::vector<std::string> terminate_commands;
   lldb::tid_t focus_tid;
   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;
   ProgressEventReporter progress_event_reporter;
   // 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;
   VSCode();
struct VSCode {
   /// \param[in] callback
   ///     The callback to execute when the given request is triggered by the
   ///     IDE.
   void RegisterRequestCallback(std::string request, RequestCallback callback);
 
   /// 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
   // JSON bytes.
   void SendJSON(const std::string &json_str);
 };
 
 extern VSCode g_vsc;
 
 } // namespace lldb_vscode
 
 #endif

lldb/tools/lldb-vscode/VSCode.cpp

: broadcaster("lldb-vscode"),
       exception_breakpoints(
           {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
            {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
            {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
            {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
            {"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");
 #if defined(_WIN32)
   // Windows opens stdout and stdin in text mode which converts \n to 13,10
   // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
   // fixes this.
   int result = _setmode(fileno(stdout), _O_BINARY);
PacketStatus VSCode::SendReverseRequest(llvm::json::Object request,
   return PacketStatus::EndOfFile;
 }
 
 void VSCode::RegisterRequestCallback(std::string request,
                                      RequestCallback callback) {
   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();
   registers.Clear();
   expandable_variables.clear();
 }
 
 int64_t Variables::GetNewVariableRefence(bool is_permanent) {

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

if (listener.WaitForEvent(1, event)) {
             break;
           case lldb::eStateCrashed:
             break;
           case lldb::eStateDetached:
             break;
           case lldb::eStateSuspended:
             break;
           case lldb::eStateStopped:
+            // 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:
             g_vsc.WillContinue();
             break;
           case lldb::eStateExited: {
             // Run any exit LLDB commands the user specified in the
             // launch.json
             g_vsc.RunExitCommands();
void request_attach(const llvm::json::Object &request) {
   attach_info.SetWaitForLaunch(wait_for, false /*async*/);
   g_vsc.init_commands = GetStrings(arguments, "initCommands");
   g_vsc.pre_run_commands = GetStrings(arguments, "preRunCommands");
   g_vsc.stop_commands = GetStrings(arguments, "stopCommands");
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   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 =
       GetStrings(arguments, "postRunCommands");
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
   // 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
void request_attach(const llvm::json::Object &request) {
   } 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
     // is no valid process after running these commands, we have failed.
     g_vsc.RunLLDBCommands("Running attachCommands:", attachCommands);
     // 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()) {
     auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
     if (attached_pid == LLDB_INVALID_PROCESS_ID) {
       if (attachCommands.empty())
         error.SetErrorString("failed to attach to a process");
       else

 //             "description": "Response to 'configurationDone' request. This is
 //             just an acknowledgement, so no body field is required."
 //             }]
 // },
 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
     g_vsc.target.GetProcess().Continue();
 }
 
 // "DisconnectRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
void request_launch(const llvm::json::Object &request) {
   g_vsc.stop_commands = GetStrings(arguments, "stopCommands");
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   g_vsc.terminate_commands = GetStrings(arguments, "terminateCommands");
   auto launchCommands = GetStrings(arguments, "launchCommands");
   std::vector<std::string> postRunCommands =
       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
   // the lldb-vscode binary to have its working directory set to that relative
   // root for the .o files in order to be able to load debug info.
   if (!debuggerRoot.empty())
     llvm::sys::fs::set_current_path(debuggerRoot);
 
if (GetBoolean(arguments, "runInTerminal", false)) {
     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 as the the launch commands are not run using the synchronous
+    // mode.
+    error = g_vsc.WaitForProcessToStop(timeout_seconds);
   }
 
   if (error.Fail()) {
     response["success"] = llvm::json::Value(false);
     EmplaceSafeString(response, "message", std::string(error.GetCString()));
   } else {
     g_vsc.RunLLDBCommands("Running postRunCommands:", postRunCommands);
   }

lldb/tools/lldb-vscode/package.json

"debuggers": [
               },
               "postRunCommands": {
                 "type": "array",
                 "description": "Commands executed just as soon as the program is successfully launched when it's in a stopped state prior to any automatic continuation.",
                 "default": []
               },
               "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": {
                 "type": "array",
                 "description": "Commands executed each time the program stops.",
                 "default": []
               },
               "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",
                 "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."
               }
             }
           },
           "attach": {
             "properties": {
               "program": {
                 "type": "string",
                 "description": "Path to the program to attach to."
"debuggers": [
               "exitCommands": {
                 "type": "array",
                 "description": "Commands executed at the end of debugging session.",
                 "default": []
               },
               "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."
               }
             }
           }
         },
         "initialConfigurations": [
           {
             "type": "lldb-vscode",
             "request": "launch",