[Lldb-commits] [PATCH] D120755: 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"...

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added inline comments.


================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:532-533
+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
----------------
The same: update the comment to reflect latest state.


================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:551
+  auto timeout_time =
+      std::chrono::high_resolution_clock::now() + std::chrono::seconds(seconds);
+  while (std::chrono::high_resolution_clock::now() < timeout_time) {
----------------
labath wrote:
> `steady_clock` would be better here (no time travel there)
Agreed.


================
Comment at: lldb/tools/lldb-vscode/VSCode.h:249-251
+  /// 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").
----------------
This comment is not true in this diff anymore. We only use async mode in launchCommands/attachCommands.


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:459
+            // event by calling the SendThreadStoppedEvent().
+            if (process.GetStopID() > 1) {
+              // Only report a stopped event if the process was not restarted.
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120755/new/

https://reviews.llvm.org/D120755