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." } } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits