Author: Walter Erquinigo Date: 2020-06-23T11:47:43-07:00 New Revision: 0a9e7d0b6befad866dfd61f05b774247e0867121
URL: https://github.com/llvm/llvm-project/commit/0a9e7d0b6befad866dfd61f05b774247e0867121 DIFF: https://github.com/llvm/llvm-project/commit/0a9e7d0b6befad866dfd61f05b774247e0867121.diff LOG: [vscode] set default values for terminateDebuggee for the disconnect request Summary: Recently I've noticed that VSCode sometimes doesn't send the terminateDebuggee flag within the disconnectRequest, even though lldb-vscode sets the terminateDebuggee capability correctly. This has been causing that inferiors don't die after the debug session ends, and many users have reported issues because of this. An easy way to mitigate this is to set better default values for the terminateDebuggee field in the disconnect request. I'm assuming that for a launch request, the default will be true, and for attach it'll be false. Reviewers: clayborg, labath, aadsm Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D81200 Added: lldb/test/API/tools/lldb-vscode/disconnect/Makefile lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py lldb/test/API/tools/lldb-vscode/disconnect/main.cpp Modified: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py lldb/tools/lldb-vscode/VSCode.cpp lldb/tools/lldb-vscode/VSCode.h lldb/tools/lldb-vscode/lldb-vscode.cpp Removed: ################################################################################ diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py index 790628d2b0fd..42eb306b9dfc 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py @@ -239,14 +239,15 @@ def continue_to_exit(self, exitCode=0): def attach(self, program=None, pid=None, waitFor=None, trace=None, initCommands=None, preRunCommands=None, stopCommands=None, - exitCommands=None, attachCommands=None, coreFile=None): + exitCommands=None, attachCommands=None, coreFile=None, disconnectAutomatically=True): '''Build the default Makefile target, create the VSCode debug adaptor, and attach to the process. ''' # Make sure we disconnect and terminate the VSCode debug adaptor even # if we throw an exception during the test case. def cleanup(): - self.vscode.request_disconnect(terminateDebuggee=True) + if disconnectAutomatically: + self.vscode.request_disconnect(terminateDebuggee=True) self.vscode.terminate() # Execute the cleanup function during test case tear down. diff --git a/lldb/test/API/tools/lldb-vscode/disconnect/Makefile b/lldb/test/API/tools/lldb-vscode/disconnect/Makefile new file mode 100644 index 000000000000..99998b20bcb0 --- /dev/null +++ b/lldb/test/API/tools/lldb-vscode/disconnect/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py b/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py new file mode 100644 index 000000000000..2a4103a344a4 --- /dev/null +++ b/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py @@ -0,0 +1,82 @@ +""" +Test lldb-vscode disconnect request +""" + + +import unittest2 +import vscode +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +import lldbvscode_testcase +import subprocess +import time +import os + + +class TestVSCode_launch(lldbvscode_testcase.VSCodeTestCaseBase): + + mydir = TestBase.compute_mydir(__file__) + source = 'main.cpp' + + def disconnect_and_assert_no_output_printed(self): + self.vscode.request_disconnect() + # verify we didn't get any input after disconnect + time.sleep(2) + output = self.get_stdout() + self.assertTrue(output is None or len(output) == 0) + + @skipIfWindows + @skipIfRemote + def test_launch(self): + """ + This test launches a process that would creates a file, but we disconnect + before the file is created, which terminates the process and thus the file is not + created. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + + # We set a breakpoint right before the side effect file is created + self.set_source_breakpoints(self.source, [line_number(self.source, '// breakpoint')]) + self.continue_to_next_stop() + + self.vscode.request_disconnect() + # verify we didn't produce the side effect file + time.sleep(1) + self.assertFalse(os.path.exists(program + ".side_effect")) + + + @skipIfWindows + @skipIfRemote + def test_attach(self): + """ + This test attaches to a process that creates a file. We attach and disconnect + before the file is created, and as the process is not terminated upon disconnection, + the file is created anyway. + """ + self.build_and_create_debug_adaptor() + program = self.getBuildArtifact("a.out") + + # Use a file as a synchronization point between test and inferior. + sync_file_path = lldbutil.append_to_process_working_directory(self, + "sync_file_%d" % (int(time.time()))) + self.addTearDownHook( + lambda: self.run_platform_command( + "rm %s" % + (sync_file_path))) + + self.process = subprocess.Popen([program, sync_file_path]) + lldbutil.wait_for_file_on_target(self, sync_file_path) + + self.attach(pid=self.process.pid, disconnectAutomatically=False) + response = self.vscode.request_evaluate("wait_for_attach = false;") + self.assertTrue(response['success']) + + # verify we haven't produced the side effect file yet + self.assertFalse(os.path.exists(program + ".side_effect")) + + self.vscode.request_disconnect() + time.sleep(2) + # verify we produced the side effect file, as the program continued after disconnecting + self.assertTrue(os.path.exists(program + ".side_effect")) diff --git a/lldb/test/API/tools/lldb-vscode/disconnect/main.cpp b/lldb/test/API/tools/lldb-vscode/disconnect/main.cpp new file mode 100644 index 000000000000..d3d7a4b7338a --- /dev/null +++ b/lldb/test/API/tools/lldb-vscode/disconnect/main.cpp @@ -0,0 +1,33 @@ +#include <chrono> +#include <cstdio> +#include <fstream> +#include <string> +#include <thread> + +volatile bool wait_for_attach = true; + +void handle_attach(char *sync_file_path) { + lldb_enable_attach(); + + { + // Create a file to signal that this process has started up. + std::ofstream sync_file; + sync_file.open(sync_file_path); + } + + while (wait_for_attach) + std::this_thread::sleep_for(std::chrono::milliseconds(10)); +} + +int main(int argc, char **args) { + if (argc == 2) + handle_attach(args[1]); + + // We let the binary live a little bit to see if it executed after detaching + // from // breakpoint + + // Create a file to signal that this process has started up. + std::ofstream out_file; // breakpoint + out_file.open(std::string(args[0]) + ".side_effect"); + return 0; +} diff --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp index 36bc8ec8ebfd..81a8e10f24fe 100644 --- a/lldb/tools/lldb-vscode/VSCode.cpp +++ b/lldb/tools/lldb-vscode/VSCode.cpp @@ -38,7 +38,7 @@ 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) { + stop_at_entry(false), is_attach(false) { 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 diff --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h index 5298d7554f4d..aceba485b68e 100644 --- a/lldb/tools/lldb-vscode/VSCode.h +++ b/lldb/tools/lldb-vscode/VSCode.h @@ -89,6 +89,7 @@ struct VSCode { lldb::tid_t focus_tid; bool sent_terminated_event; bool stop_at_entry; + bool is_attach; // 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; diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp index f1620d945fbc..3ec4c2c070af 100644 --- a/lldb/tools/lldb-vscode/lldb-vscode.cpp +++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -514,6 +514,7 @@ void SetSourceMapFromArguments(const llvm::json::Object &arguments) { // }] // } void request_attach(const llvm::json::Object &request) { + g_vsc.is_attach = true; llvm::json::Object response; lldb::SBError error; FillResponse(request, response); @@ -769,7 +770,9 @@ void request_disconnect(const llvm::json::Object &request) { FillResponse(request, response); auto arguments = request.getObject("arguments"); - bool terminateDebuggee = GetBoolean(arguments, "terminateDebuggee", false); + bool defaultTerminateDebuggee = g_vsc.is_attach ? false : true; + bool terminateDebuggee = + GetBoolean(arguments, "terminateDebuggee", defaultTerminateDebuggee); lldb::SBProcess process = g_vsc.target.GetProcess(); auto state = process.GetState(); @@ -788,10 +791,9 @@ void request_disconnect(const llvm::json::Object &request) { case lldb::eStateStopped: case lldb::eStateRunning: g_vsc.debugger.SetAsync(false); - if (terminateDebuggee) - process.Kill(); - else - process.Detach(); + lldb::SBError error = terminateDebuggee ? process.Kill() : process.Detach(); + if (!error.Success()) + response.try_emplace("error", error.GetCString()); g_vsc.debugger.SetAsync(true); break; } @@ -1357,6 +1359,7 @@ void request_initialize(const llvm::json::Object &request) { // }] // } void request_launch(const llvm::json::Object &request) { + g_vsc.is_attach = false; llvm::json::Object response; lldb::SBError error; FillResponse(request, response); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
