diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index c974866306d2a..4cf2da2dceb83 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -7,6 +7,7 @@ import pprint import socket import string +import signal import subprocess import sys import threading @@ -97,37 +98,15 @@ def dump_dap_log(log_file): print("========= END =========", file=sys.stderr) -def read_packet_thread(vs_comm, log_file): - done = False - try: - while not done: - packet = read_packet(vs_comm.recv, trace_file=vs_comm.trace_file) - # `packet` will be `None` on EOF. We want to pass it down to - # handle_recv_packet anyway so the main thread can handle unexpected - # termination of lldb-dap and stop waiting for new packets. - done = not vs_comm.handle_recv_packet(packet) - finally: - # Wait for the process to fully exit before dumping the log file to - # ensure we have the entire log contents. - if vs_comm.process is not None: - try: - # Do not wait forever, some logs are better than none. - vs_comm.process.wait(timeout=20) - except subprocess.TimeoutExpired: - pass - dump_dap_log(log_file) - - class DebugCommunication(object): def __init__(self, recv, send, init_commands, log_file=None): + self.log_file = log_file self.trace_file = None self.send = send self.recv = recv self.recv_packets = [] self.recv_condition = threading.Condition() - self.recv_thread = threading.Thread( - target=read_packet_thread, args=(self, log_file) - ) + self.recv_thread = threading.Thread(target=self._read_packet_thread) self.process_event_body = None self.exit_status = None self.initialize_body = None @@ -155,6 +134,15 @@ def validate_response(cls, command, response): if command["seq"] != response["request_seq"]: raise ValueError("seq mismatch in response") + def _read_packet_thread(self): + done = False + while not done: + packet = read_packet(self.recv, trace_file=self.trace_file) + # `packet` will be `None` on EOF. We want to pass it down to + # handle_recv_packet anyway so the main thread can handle unexpected + # termination of lldb-dap and stop waiting for new packets. + done = not self.handle_recv_packet(packet) + def get_modules(self): module_list = self.request_modules()["body"]["modules"] modules = {} @@ -247,10 +235,12 @@ def handle_recv_packet(self, packet): # and 'progressEnd' events. Keep these around in case test # cases want to verify them. self.progress_events.append(packet) - - elif packet_type == "response": - if packet["command"] == "disconnect": + elif event == "terminated": + # The terminated event corresponds to the last message we expect + # on the DAP. See section 'Debug session end' on + # https://microsoft.github.io/debug-adapter-protocol/overview keepGoing = False + self.enqueue_recv_packet(packet) return keepGoing @@ -307,7 +297,6 @@ def recv_packet(self, filter_type=None, filter_event=None, timeout=None): return None finally: self.recv_condition.release() - return None def send_recv(self, command): @@ -1263,7 +1252,7 @@ def launch(cls, /, executable, env=None, log_file=None, connection=None): args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stderr=sys.stderr, env=adapter_env, ) @@ -1294,17 +1283,42 @@ def get_pid(self): return -1 def terminate(self): - super(DebugAdapterServer, self).terminate() if self.process is not None: - self.process.terminate() + process = self.process + self.process = None try: - self.process.wait(timeout=20) + # When we close stdin it should signal the lldb-dap that no + # new messages will arrive and it should shutdown on its own. + process.stdin.close() + process.wait(timeout=30.0) except subprocess.TimeoutExpired: - self.process.kill() - self.process.wait() - self.process = None + process.kill() + process.wait() + finally: + dump_dap_log(self.log_file) + if process.returncode != 0: + raise DebugAdapterProcessError(process.returncode) +class DebugAdapterError(Exception): + pass + + +class DebugAdapterProcessError(DebugAdapterError): + """Raised when the lldb-dap process exits with a non-zero exit status.""" + + def __init__(self, returncode): + self.returncode = returncode + + def __str__(self): + if self.returncode and self.returncode < 0: + try: + return f"lldb-dap died with {signal.Signals(-self.returncode).name}." + except ValueError: + return f"lldb-dap died with unknown signal {-self.returncode}." + else: + return f"lldb-dap returned non-zero exit status {self.returncode}." + def attach_options_specified(options): if options.pid is not None: return True diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index c5a7eb76a58c7..63b3827c2221c 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -146,7 +146,9 @@ def verify_commands(self, flavor, output, commands): found = True break self.assertTrue( - found, "verify '%s' found in console output for '%s'" % (cmd, flavor) + found, + "verify '%s' found in console output for '%s' in %s" + % (cmd, flavor, output), ) def get_dict_value(self, d, key_path): diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py index a9218d3c3dde3..e64acf968ed38 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -139,7 +139,9 @@ def test_commands(self): exitCommands=exitCommands, terminateCommands=terminateCommands, postRunCommands=postRunCommands, + disconnectAutomatically=False, ) + self.dap_server.wait_for_stopped() # Get output from the console. This should contain both the # "initCommands" and the "preRunCommands". output = self.get_console() @@ -169,6 +171,9 @@ def test_commands(self): # Continue until the program exits self.continue_to_exit() + + self.dap_server.request_disconnect(terminateDebuggee=True) + # Get output from the console. This should contain both the # "exitCommands" that were run after the second breakpoint was hit # and the "terminateCommands" due to the debugging session ending diff --git a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py index 479a91208a66c..03e15bbcb5337 100644 --- a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py +++ b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py @@ -45,7 +45,7 @@ def test_pending_request(self): """ program = self.getBuildArtifact("a.out") self.build_and_launch(program, stopOnEntry=True) - self.continue_to_next_stop() + self.dap_server.wait_for_stopped() # Use a relatively short timeout since this is only to ensure the # following request is queued. @@ -78,7 +78,7 @@ def test_inflight_request(self): """ program = self.getBuildArtifact("a.out") self.build_and_launch(program, stopOnEntry=True) - self.continue_to_next_stop() + self.dap_server.wait_for_stopped() blocking_seq = self.async_blocking_request(duration=self.timeoutval / 2) # Wait for the sleep to start to cancel the inflight request. diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py index 8642e317f9b3a..e635529be92c0 100644 --- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py +++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py @@ -169,6 +169,7 @@ def test_exit_status_message_ok(self): def test_diagnositcs(self): program = self.getBuildArtifact("a.out") self.build_and_launch(program, stopOnEntry=True) + self.dap_server.wait_for_stopped() core = self.getBuildArtifact("minidump.core") self.yaml2obj("minidump.yaml", core) @@ -176,7 +177,9 @@ def test_diagnositcs(self): f"target create --core {core}", context="repl" ) - output = self.get_important() + output = self.collect_important( + self.timeoutval, pattern="process ID from minidump file" + ) self.assertIn( "warning: unable to retrieve process ID from minidump file", output, diff --git a/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py index f05f876e57b49..e1af4a70e45f1 100644 --- a/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py +++ b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py @@ -22,13 +22,9 @@ def cleanup(): process.terminate() process.wait() stdout_data = process.stdout.read().decode() - stderr_data = process.stderr.read().decode() print("========= STDOUT =========", file=sys.stderr) print(stdout_data, file=sys.stderr) print("========= END =========", file=sys.stderr) - print("========= STDERR =========", file=sys.stderr) - print(stderr_data, file=sys.stderr) - print("========= END =========", file=sys.stderr) print("========= DEBUG ADAPTER PROTOCOL LOGS =========", file=sys.stderr) with open(log_file_path, "r") as file: print(file.read(), file=sys.stderr) @@ -52,13 +48,13 @@ def test_invalid_header(self): lldb-dap handles invalid message headers. """ process = self.launch() - process.stdin.write(b"not the corret message header") + process.stdin.write(b"not the correct message header") process.stdin.close() self.assertEqual(process.wait(timeout=5.0), 1) def test_partial_header(self): """ - lldb-dap handles parital message headers. + lldb-dap handles partial message headers. """ process = self.launch() process.stdin.write(b"Content-Length: ") diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 7c85f05c1ba45..3332fe2690036 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -75,9 +75,7 @@ def test_termination(self): self.dap_server.request_disconnect() # Wait until the underlying lldb-dap process dies. - self.dap_server.process.wait( - timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval - ) + self.dap_server.process.wait(timeout=self.timeoutval) # Check the return code self.assertEqual(self.dap_server.process.poll(), 0) @@ -536,12 +534,13 @@ def test_terminate_commands(self): terminateCommands=terminateCommands, disconnectAutomatically=False, ) + self.dap_server.wait_for_stopped() self.get_console() # Once it's disconnected the console should contain the # "terminateCommands" self.dap_server.request_disconnect(terminateDebuggee=True) output = self.collect_console( - timeout_secs=1.0, + timeout_secs=self.timeoutval, pattern=terminateCommands[0], ) self.verify_commands("terminateCommands", output, terminateCommands) @@ -553,7 +552,7 @@ def test_version(self): as the one returned by "version" command. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program) + self.build_and_launch(program, stopOnEntry=True) source = "main.c" breakpoint_line = line_number(source, "// breakpoint 1") diff --git a/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py index b0abe2a38dac4..94507d512988f 100644 --- a/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py +++ b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py @@ -32,7 +32,7 @@ def test_terminated_event(self): program_basename = "a.out.stripped" program = self.getBuildArtifact(program_basename) - self.build_and_launch(program) + self.build_and_launch(program, stopOnEntry=True, disconnectAutomatically=False) # Set breakpoints functions = ["foo"] breakpoint_ids = self.set_function_breakpoints(functions) diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp index 26d633d1d172e..a3da238c8ea5d 100644 --- a/lldb/tools/lldb-dap/Breakpoint.cpp +++ b/lldb/tools/lldb-dap/Breakpoint.cpp @@ -7,16 +7,13 @@ //===----------------------------------------------------------------------===// #include "Breakpoint.h" -#include "DAP.h" #include "JSONUtils.h" #include "lldb/API/SBAddress.h" #include "lldb/API/SBBreakpointLocation.h" #include "lldb/API/SBLineEntry.h" -#include "lldb/API/SBMutex.h" #include "llvm/ADT/StringExtras.h" #include #include -#include #include using namespace lldb_dap; @@ -81,9 +78,6 @@ bool Breakpoint::MatchesName(const char *name) { } void Breakpoint::SetBreakpoint() { - lldb::SBMutex lock = m_dap.GetAPIMutex(); - std::lock_guard guard(lock); - m_bp.AddName(kDAPBreakpointLabel); if (!m_condition.empty()) SetCondition(); diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 4feca1253be20..f8140b9b9fc70 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -21,11 +21,14 @@ #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBCommandInterpreter.h" #include "lldb/API/SBCommandReturnObject.h" +#include "lldb/API/SBDefines.h" #include "lldb/API/SBEvent.h" #include "lldb/API/SBLanguageRuntime.h" #include "lldb/API/SBListener.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" +#include "lldb/Host/MainLoop.h" +#include "lldb/Host/MainLoopBase.h" #include "lldb/Utility/IOObject.h" #include "lldb/Utility/Status.h" #include "lldb/lldb-defines.h" @@ -253,13 +256,13 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { } void DAP::StopEventHandlers() { - if (event_thread.joinable()) { + if (m_event_thread.joinable()) { broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); - event_thread.join(); + m_event_thread.join(); } - if (progress_event_thread.joinable()) { + if (m_progress_event_thread.joinable()) { broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread); - progress_event_thread.join(); + m_progress_event_thread.join(); } } @@ -737,7 +740,7 @@ void DAP::SetTarget(const lldb::SBTarget target) { } } -bool DAP::HandleObject(const Message &M) { +bool DAP::HandleProtocolMessage(const Message &M) { TelemetryDispatcher dispatcher(&debugger); dispatcher.Set("client_name", transport.GetClientName().str()); if (const auto *req = std::get_if(&M)) { @@ -861,9 +864,10 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) { } } - SendTerminatedEvent(); + RequestTermination(); - disconnecting = true; + // Stop event handlers to ensure we flush any events caused by kill/detach. + StopEventHandlers(); return ToError(error); } @@ -895,112 +899,110 @@ static std::optional getArgumentsIfRequest(const Message &pm, return std::move(args); } -llvm::Error DAP::Loop() { - // Can't use \a std::future because it doesn't compile on - // Windows. - std::future queue_reader = - std::async(std::launch::async, [&]() -> lldb::SBError { - llvm::set_thread_name(transport.GetClientName() + ".transport_handler"); - auto cleanup = llvm::make_scope_exit([&]() { - // Ensure we're marked as disconnecting when the reader exits. - disconnecting = true; - m_queue_cv.notify_all(); - }); - - while (!disconnecting) { - llvm::Expected next = - transport.Read(std::chrono::seconds(1)); - if (next.errorIsA()) { - consumeError(next.takeError()); - break; - } - - // If the read timed out, continue to check if we should disconnect. - if (next.errorIsA()) { - consumeError(next.takeError()); - continue; - } - - if (llvm::Error err = next.takeError()) { - lldb::SBError errWrapper; - errWrapper.SetErrorString(llvm::toString(std::move(err)).c_str()); - return errWrapper; - } - - // The launch sequence is special and we need to carefully handle - // packets in the right order. Until we've handled configurationDone, - bool add_to_pending_queue = false; - - if (const protocol::Request *req = - std::get_if(&*next)) { - llvm::StringRef command = req->command; - if (command == "disconnect") - disconnecting = true; - if (!configuration_done) - add_to_pending_queue = - command != "initialize" && command != "configurationDone" && - command != "disconnect" && !command.ends_with("Breakpoints"); - } - - const std::optional cancel_args = - getArgumentsIfRequest(*next, "cancel"); - if (cancel_args) { - { - std::lock_guard guard(m_cancelled_requests_mutex); - if (cancel_args->requestId) - m_cancelled_requests.insert(*cancel_args->requestId); - } - - // If a cancel is requested for the active request, make a best - // effort attempt to interrupt. - std::lock_guard guard(m_active_request_mutex); - if (m_active_request && - cancel_args->requestId == m_active_request->seq) { - DAP_LOG( - log, - "({0}) interrupting inflight request (command={1} seq={2})", - transport.GetClientName(), m_active_request->command, - m_active_request->seq); - debugger.RequestInterrupt(); - } - } - - { - std::lock_guard guard(m_queue_mutex); - auto &queue = add_to_pending_queue ? m_pending_queue : m_queue; - queue.push_back(std::move(*next)); - } - m_queue_cv.notify_one(); - } - - return lldb::SBError(); - }); - +llvm::Error DAP::Run() { + std::future transport_error = + std::async(std::launch::async, &DAP::TransportThread, this); auto cleanup = llvm::make_scope_exit([&]() { + // Ensure the terminated event has been sent by this point, in case the + // debuggee exits or client disconnects during the initialization launch + // flow. + SendTerminatedEvent(); out.Stop(); err.Stop(); StopEventHandlers(); + if (debugger.IsValid()) + lldb::SBDebugger::Destroy(debugger); }); - while (true) { - std::unique_lock lock(m_queue_mutex); - m_queue_cv.wait(lock, [&] { return disconnecting || !m_queue.empty(); }); + lldb_private::Status status = m_loop.Run(); + if (auto err = status.takeError()) + return err; - if (disconnecting && m_queue.empty()) + transport_error.wait(); + return ToError(transport_error.get()); +} + +lldb::SBError DAP::TransportThread() { + llvm::set_thread_name(transport.GetClientName() + ".transport_handler"); + auto cleanup = llvm::make_scope_exit([&] { RequestTermination(); }); + while (!disconnecting) { + llvm::Expected next = transport.Read(std::chrono::seconds(1)); + if (next.errorIsA()) { + consumeError(next.takeError()); break; + } - Message next = m_queue.front(); - m_queue.pop_front(); + // If the read timed out, continue to check if we should disconnect. + if (next.errorIsA()) { + consumeError(next.takeError()); + continue; + } + + if (llvm::Error err = next.takeError()) { + DAP_LOG(log, "({0}) transport read failed: {1}", + transport.GetClientName(), llvm::toStringWithoutConsuming(err)); + lldb::SBError errWrapper; + errWrapper.SetErrorString(llvm::toString(std::move(err)).c_str()); + return errWrapper; + } + + // The launch sequence is special and we need to carefully handle + // packets in the right order. Until we've handled configurationDone, + bool add_to_pending_queue = false; + + if (const protocol::Request *req = std::get_if(&*next)) { + llvm::StringRef command = req->command; + if (command == "disconnect") + disconnecting = true; + if (!configuration_done) + add_to_pending_queue = + command != "initialize" && command != "configurationDone" && + command != "disconnect" && !command.ends_with("Breakpoints"); + } - // Unlock while we're processing the event. - lock.unlock(); + const std::optional cancel_args = + getArgumentsIfRequest(*next, "cancel"); + if (cancel_args) { + { + std::lock_guard guard(m_cancelled_requests_mutex); + if (cancel_args->requestId) + m_cancelled_requests.insert(*cancel_args->requestId); + } + + // If a cancel is requested for the active request, make a best + // effort attempt to interrupt. + std::lock_guard guard(m_active_request_mutex); + if (m_active_request && cancel_args->requestId == m_active_request->seq) { + DAP_LOG(log, + "({0}) interrupting inflight request (command={1} seq={2})", + transport.GetClientName(), m_active_request->command, + m_active_request->seq); + debugger.RequestInterrupt(); + } + } - if (!HandleObject(next)) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "unhandled packet"); + lldb_private::MainLoop::Callback handle_request = + [&, message = std::move(*next)](lldb_private::MainLoopBase &) { + if (!HandleProtocolMessage(message)) { + DAP_LOG(log, "({0}) unhandled packet", transport.GetClientName()); + RequestTermination(); + } + }; + + if (add_to_pending_queue) { + std::lock_guard guard(m_pending_queue_mutex); + m_pending_queue.push_front(handle_request); + } else + m_loop.AddPendingCallback(handle_request); } - return ToError(queue_reader.get()); + return lldb::SBError(); +} + +void DAP::RequestTermination() { + disconnecting = true; + m_loop.AddPendingCallback( + [&](lldb_private::MainLoopBase &ml) { ml.RequestTermination(); }); } lldb::SBError DAP::WaitForProcessToStop(std::chrono::seconds seconds) { @@ -1010,10 +1012,21 @@ lldb::SBError DAP::WaitForProcessToStop(std::chrono::seconds seconds) { error.SetErrorString("invalid process"); return error; } + + if (lldb::SBDebugger::StateIsStoppedState(process.GetState())) + return lldb::SBError(); // Already stopped! + + lldb::SBEvent event; + lldb::SBListener listener("dap.process.waiter"); 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(); + if (!listener.WaitForEventForBroadcasterWithType( + 1, process.GetBroadcaster(), + lldb::SBProcess::eBroadcastBitStateChanged, event)) + continue; + + auto state = lldb::SBProcess::GetStateFromEvent(event); switch (state) { case lldb::eStateUnloaded: case lldb::eStateAttaching: @@ -1034,8 +1047,8 @@ lldb::SBError DAP::WaitForProcessToStop(std::chrono::seconds seconds) { case lldb::eStateStopped: return lldb::SBError(); // Success! } - std::this_thread::sleep_for(std::chrono::microseconds(250)); } + error.SetErrorString( llvm::formatv("process failed to stop within {0}", seconds) .str() @@ -1268,13 +1281,10 @@ void DAP::SetConfiguration(const protocol::Configuration &config, } void DAP::SetConfigurationDone() { - { - std::lock_guard guard(m_queue_mutex); - std::copy(m_pending_queue.begin(), m_pending_queue.end(), - std::front_inserter(m_queue)); - configuration_done = true; - } - m_queue_cv.notify_all(); + std::lock_guard guard(m_pending_queue_mutex); + for (const auto &pending_request_handler : m_pending_queue) + m_loop.AddPendingCallback(pending_request_handler); + configuration_done = true; } void DAP::SetFrameFormat(llvm::StringRef format) { @@ -1422,11 +1432,11 @@ protocol::Capabilities DAP::GetCapabilities() { } void DAP::StartEventThread() { - event_thread = std::thread(&DAP::EventThread, this); + m_event_thread = std::thread(&DAP::EventThread, this); } void DAP::StartProgressEventThread() { - progress_event_thread = std::thread(&DAP::ProgressEventThread, this); + m_progress_event_thread = std::thread(&DAP::ProgressEventThread, this); } void DAP::ProgressEventThread() { @@ -1501,155 +1511,161 @@ void DAP::EventThread() { broadcaster.AddListener(listener, eBroadcastBitStopEventThread); debugger.GetBroadcaster().AddListener( listener, lldb::eBroadcastBitError | lldb::eBroadcastBitWarning); - bool done = false; - while (!done) { + while (true) { if (listener.WaitForEvent(1, event)) { - const auto event_mask = event.GetType(); - if (lldb::SBProcess::EventIsProcessEvent(event)) { - lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event); - if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { - auto state = lldb::SBProcess::GetStateFromEvent(event); - switch (state) { - case lldb::eStateConnected: - case lldb::eStateDetached: - case lldb::eStateInvalid: - case lldb::eStateUnloaded: - break; - case lldb::eStateAttaching: - case lldb::eStateCrashed: - case lldb::eStateLaunching: - case lldb::eStateStopped: - case lldb::eStateSuspended: - // Only report a stopped event if the process was not - // automatically restarted. - if (!lldb::SBProcess::GetRestartedFromEvent(event)) { - SendStdOutStdErr(*this, process); - SendThreadStoppedEvent(*this); - } - break; - case lldb::eStateRunning: - case lldb::eStateStepping: - WillContinue(); - SendContinuedEvent(*this); - break; - case lldb::eStateExited: - lldb::SBStream stream; - process.GetStatus(stream); - SendOutput(OutputType::Console, stream.GetData()); - - // When restarting, we can get an "exited" event for the process we - // just killed with the old PID, or even with no PID. In that case - // we don't have to terminate the session. - if (process.GetProcessID() == LLDB_INVALID_PROCESS_ID || - process.GetProcessID() == restarting_process_id) { - restarting_process_id = LLDB_INVALID_PROCESS_ID; - } else { - // Run any exit LLDB commands the user specified in the - // launch.json - RunExitCommands(); - SendProcessExitedEvent(*this, process); - SendTerminatedEvent(); - done = true; - } - break; - } - } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) || - (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) { - SendStdOutStdErr(*this, process); + if (event.BroadcasterMatchesRef(broadcaster)) { + const auto event_mask = event.GetType(); + if (event_mask & eBroadcastBitStopEventThread) { + return; } - } else if (lldb::SBTarget::EventIsTargetEvent(event)) { - if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded || - event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded || - event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded || - event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) { - const uint32_t num_modules = - lldb::SBTarget::GetNumModulesFromEvent(event); - std::lock_guard guard(modules_mutex); - for (uint32_t i = 0; i < num_modules; ++i) { - lldb::SBModule module = - lldb::SBTarget::GetModuleAtIndexFromEvent(i, event); - if (!module.IsValid()) - continue; - llvm::StringRef module_id = module.GetUUIDString(); - if (module_id.empty()) - continue; - - llvm::StringRef reason; - bool id_only = false; - if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) { - modules.insert(module_id); - reason = "new"; - } else { - // If this is a module we've never told the client about, don't - // send an event. - if (!modules.contains(module_id)) - continue; - - if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) { - modules.erase(module_id); - reason = "removed"; - id_only = true; - } else { - reason = "changed"; - } - } - - llvm::json::Object body; - body.try_emplace("reason", reason); - body.try_emplace("module", CreateModule(target, module, id_only)); - llvm::json::Object module_event = CreateEventObject("module"); - module_event.try_emplace("body", std::move(body)); - SendJSON(llvm::json::Value(std::move(module_event))); - } + } + if (event.IsValid()) + m_loop.AddPendingCallback( + [&, event = std::move(event)](auto &L) { HandleEvent(event); }); + } + } +} + +void DAP::HandleEvent(lldb::SBEvent event) { + const auto event_mask = event.GetType(); + if (lldb::SBProcess::EventIsProcessEvent(event)) { + lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event); + if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { + auto state = lldb::SBProcess::GetStateFromEvent(event); + switch (state) { + case lldb::eStateConnected: + case lldb::eStateInvalid: + case lldb::eStateUnloaded: + break; + case lldb::eStateAttaching: + case lldb::eStateCrashed: + case lldb::eStateLaunching: + case lldb::eStateStopped: + case lldb::eStateSuspended: + // Only report a stopped event if the process was not + // automatically restarted. + if (!lldb::SBProcess::GetRestartedFromEvent(event)) { + SendStdOutStdErr(*this, process); + SendThreadStoppedEvent(*this); } - } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) { - if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) { - auto event_type = - lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); - auto bp = Breakpoint( - *this, lldb::SBBreakpoint::GetBreakpointFromEvent(event)); - // If the breakpoint was set through DAP, it will have the - // BreakpointBase::kDAPBreakpointLabel. Regardless of whether - // locations were added, removed, or resolved, the breakpoint isn't - // going away and the reason is always "changed". - if ((event_type & lldb::eBreakpointEventTypeLocationsAdded || - event_type & lldb::eBreakpointEventTypeLocationsRemoved || - event_type & lldb::eBreakpointEventTypeLocationsResolved) && - bp.MatchesName(BreakpointBase::kDAPBreakpointLabel)) { - // As the DAP client already knows the path of this breakpoint, we - // don't need to send it back as part of the "changed" event. This - // avoids sending paths that should be source mapped. Note that - // CreateBreakpoint doesn't apply source mapping and certain - // implementation ignore the source part of this event anyway. - llvm::json::Value source_bp = bp.ToProtocolBreakpoint(); - source_bp.getAsObject()->erase("source"); - - llvm::json::Object body; - body.try_emplace("breakpoint", source_bp); - body.try_emplace("reason", "changed"); - - llvm::json::Object bp_event = CreateEventObject("breakpoint"); - bp_event.try_emplace("body", std::move(body)); - - SendJSON(llvm::json::Value(std::move(bp_event))); - } + break; + case lldb::eStateRunning: + case lldb::eStateStepping: + WillContinue(); + SendContinuedEvent(*this); + break; + case lldb::eStateDetached: + case lldb::eStateExited: + lldb::SBStream stream; + process.GetStatus(stream); + SendOutput(OutputType::Console, stream.GetData()); + + // When restarting, we can get an "exited" event for the process we + // just killed with the old PID, or even with no PID. In that case + // we don't have to terminate the session. + if (process.GetProcessID() == LLDB_INVALID_PROCESS_ID || + process.GetProcessID() == restarting_process_id) { + restarting_process_id = LLDB_INVALID_PROCESS_ID; + } else { + // Run any exit LLDB commands the user specified in the + // launch.json + RunExitCommands(); + SendProcessExitedEvent(*this, process); + RequestTermination(); } - } else if (event_mask & lldb::eBroadcastBitError || - event_mask & lldb::eBroadcastBitWarning) { - lldb::SBStructuredData data = - lldb::SBDebugger::GetDiagnosticFromEvent(event); - if (!data.IsValid()) + break; + } + } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) || + (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) { + SendStdOutStdErr(*this, process); + } + } else if (lldb::SBTarget::EventIsTargetEvent(event)) { + if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded || + event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded || + event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded || + event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) { + const uint32_t num_modules = + lldb::SBTarget::GetNumModulesFromEvent(event); + std::lock_guard guard(modules_mutex); + for (uint32_t i = 0; i < num_modules; ++i) { + lldb::SBModule module = + lldb::SBTarget::GetModuleAtIndexFromEvent(i, event); + if (!module.IsValid()) continue; - std::string type = GetStringValue(data.GetValueForKey("type")); - std::string message = GetStringValue(data.GetValueForKey("message")); - SendOutput(OutputType::Important, - llvm::formatv("{0}: {1}", type, message).str()); - } else if (event.BroadcasterMatchesRef(broadcaster)) { - if (event_mask & eBroadcastBitStopEventThread) { - done = true; + llvm::StringRef module_id = module.GetUUIDString(); + if (module_id.empty()) + continue; + + llvm::StringRef reason; + bool id_only = false; + if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) { + modules.insert(module_id); + reason = "new"; + } else { + // If this is a module we've never told the client about, don't + // send an event. + if (!modules.contains(module_id)) + continue; + + if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) { + modules.erase(module_id); + reason = "removed"; + id_only = true; + } else { + reason = "changed"; + } } + + llvm::json::Object body; + body.try_emplace("reason", reason); + body.try_emplace("module", CreateModule(target, module, id_only)); + llvm::json::Object module_event = CreateEventObject("module"); + module_event.try_emplace("body", std::move(body)); + SendJSON(llvm::json::Value(std::move(module_event))); + } + } + } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) { + if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) { + auto event_type = + lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); + auto bp = + Breakpoint(*this, lldb::SBBreakpoint::GetBreakpointFromEvent(event)); + // If the breakpoint was set through DAP, it will have the + // BreakpointBase::kDAPBreakpointLabel. Regardless of whether + // locations were added, removed, or resolved, the breakpoint isn't + // going away and the reason is always "changed". + if ((event_type & lldb::eBreakpointEventTypeLocationsAdded || + event_type & lldb::eBreakpointEventTypeLocationsRemoved || + event_type & lldb::eBreakpointEventTypeLocationsResolved) && + bp.MatchesName(BreakpointBase::kDAPBreakpointLabel)) { + // As the DAP client already knows the path of this breakpoint, we + // don't need to send it back as part of the "changed" event. This + // avoids sending paths that should be source mapped. Note that + // CreateBreakpoint doesn't apply source mapping and certain + // implementation ignore the source part of this event anyway. + llvm::json::Value source_bp = bp.ToProtocolBreakpoint(); + source_bp.getAsObject()->erase("source"); + + llvm::json::Object body; + body.try_emplace("breakpoint", source_bp); + body.try_emplace("reason", "changed"); + + llvm::json::Object bp_event = CreateEventObject("breakpoint"); + bp_event.try_emplace("body", std::move(body)); + + SendJSON(llvm::json::Value(std::move(bp_event))); } } + } else if (event_mask & lldb::eBroadcastBitError || + event_mask & lldb::eBroadcastBitWarning) { + lldb::SBStructuredData data = + lldb::SBDebugger::GetDiagnosticFromEvent(event); + if (!data.IsValid()) + return; + std::string type = GetStringValue(data.GetValueForKey("type")); + std::string message = GetStringValue(data.GetValueForKey("message")); + SendOutput(OutputType::Important, + llvm::formatv("{0}: {1}", type, message).str()); } } diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index c2e4c2dea582e..9797cbf83cd33 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -32,6 +32,7 @@ #include "lldb/API/SBThread.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" +#include "lldb/Host/MainLoop.h" #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -46,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -338,7 +340,7 @@ struct DAP { /// listeing for its breakpoint events. void SetTarget(const lldb::SBTarget target); - bool HandleObject(const protocol::Message &M); + bool HandleProtocolMessage(const protocol::Message &M); /// Disconnect the DAP session. llvm::Error Disconnect(); @@ -349,7 +351,11 @@ struct DAP { /// Send a "terminated" event to indicate the process is done being debugged. void SendTerminatedEvent(); - llvm::Error Loop(); + /// Run the main run loop for the debug adapter session. + llvm::Error Run(); + + /// Stop the main run loop for the debug adapter session. + void RequestTermination(); /// Send a Debug Adapter Protocol reverse request to the IDE. /// @@ -417,8 +423,6 @@ struct DAP { /// Clears the cancel request from the set of tracked cancel requests. void ClearCancelRequest(const protocol::CancelArguments &); - lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); } - void StartEventThread(); void StartProgressEventThread(); @@ -435,17 +439,20 @@ struct DAP { /// Event threads. /// @{ void EventThread(); + void HandleEvent(lldb::SBEvent event); void ProgressEventThread(); - std::thread event_thread; - std::thread progress_event_thread; + std::thread m_event_thread; + std::thread m_progress_event_thread; /// @} + lldb::SBError TransportThread(); + std::thread m_transport_thread; + lldb_private::MainLoop m_loop; + /// Queue for all incoming messages. - std::deque m_queue; - std::deque m_pending_queue; - std::mutex m_queue_mutex; - std::condition_variable m_queue_cv; + std::deque m_pending_queue; + std::mutex m_pending_queue_mutex; std::mutex m_cancelled_requests_mutex; llvm::SmallSet m_cancelled_requests; diff --git a/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp b/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp index 9772e7344ced6..d8109daf89129 100644 --- a/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp +++ b/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp @@ -9,16 +9,11 @@ #include "ExceptionBreakpoint.h" #include "BreakpointBase.h" #include "DAP.h" -#include "lldb/API/SBMutex.h" #include "lldb/API/SBTarget.h" -#include namespace lldb_dap { void ExceptionBreakpoint::SetBreakpoint() { - lldb::SBMutex lock = m_dap.GetAPIMutex(); - std::lock_guard guard(lock); - if (m_bp.IsValid()) return; bool catch_value = m_filter.find("_catch") != std::string::npos; diff --git a/lldb/tools/lldb-dap/FunctionBreakpoint.cpp b/lldb/tools/lldb-dap/FunctionBreakpoint.cpp index 1ea9cddb9f689..78c5977b42776 100644 --- a/lldb/tools/lldb-dap/FunctionBreakpoint.cpp +++ b/lldb/tools/lldb-dap/FunctionBreakpoint.cpp @@ -8,8 +8,6 @@ #include "FunctionBreakpoint.h" #include "DAP.h" -#include "lldb/API/SBMutex.h" -#include namespace lldb_dap { @@ -19,9 +17,6 @@ FunctionBreakpoint::FunctionBreakpoint( m_function_name(breakpoint.name) {} void FunctionBreakpoint::SetBreakpoint() { - lldb::SBMutex lock = m_dap.GetAPIMutex(); - std::lock_guard guard(lock); - if (m_function_name.empty()) return; m_bp = m_dap.target.BreakpointCreateByName(m_function_name.c_str()); diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index 93bc80a38e29d..17e3d5e4692dc 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -18,7 +18,6 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBEnvironment.h" #include "llvm/Support/Error.h" -#include #if !defined(_WIN32) #include @@ -138,9 +137,6 @@ void BaseRequestHandler::Run(const Request &request) { return; } - lldb::SBMutex lock = dap.GetAPIMutex(); - std::lock_guard guard(lock); - // FIXME: After all the requests have migrated from LegacyRequestHandler > // RequestHandler<> we should be able to move this into // RequestHandler<>::operator(). diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.cpp b/lldb/tools/lldb-dap/SourceBreakpoint.cpp index 4581c995b4260..bee5a8353ab07 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.cpp +++ b/lldb/tools/lldb-dap/SourceBreakpoint.cpp @@ -13,7 +13,6 @@ #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBFileSpecList.h" #include "lldb/API/SBFrame.h" -#include "lldb/API/SBMutex.h" #include "lldb/API/SBTarget.h" #include "lldb/API/SBThread.h" #include "lldb/API/SBValue.h" @@ -21,7 +20,6 @@ #include #include #include -#include #include namespace lldb_dap { @@ -34,9 +32,6 @@ SourceBreakpoint::SourceBreakpoint(DAP &dap, m_column(breakpoint.column.value_or(LLDB_INVALID_COLUMN_NUMBER)) {} void SourceBreakpoint::SetBreakpoint(const llvm::StringRef source_path) { - lldb::SBMutex lock = m_dap.GetAPIMutex(); - std::lock_guard guard(lock); - lldb::SBFileSpecList module_list; m_bp = m_dap.target.BreakpointCreateByLocation( source_path.str().c_str(), m_line, m_column, 0, module_list); diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h index 4e347eaa51314..65a07ac995ff0 100644 --- a/lldb/tools/lldb-dap/Transport.h +++ b/lldb/tools/lldb-dap/Transport.h @@ -66,14 +66,14 @@ class Transport { void operator=(const Transport &rhs) = delete; /// @} - /// Writes a Debug Adater Protocol message to the output stream. + /// Writes a Debug Adapter Protocol message to the output stream. llvm::Error Write(const protocol::Message &M); - /// Reads the next Debug Adater Protocol message from the input stream. + /// Reads the next Debug Adapter Protocol message from the input stream. /// /// \param timeout[in] /// A timeout to wait for reading the initial header. Once a message - /// header is recieved, this will block until the full message is + /// header is received, this will block until the full message is /// read. /// /// \returns Returns the next protocol message. diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index 7a4cc70902a56..f7033e6c17ab3 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -305,7 +305,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, dap_sessions[io.get()] = &dap; } - if (auto Err = dap.Loop()) { + if (auto Err = dap.Run()) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "DAP session (" + client_name + ") error: "); @@ -342,8 +342,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, << " disconnected failed: " << llvm::toString(std::move(error)) << "\n"; } - // Close the socket to ensure the DAP::Loop read finishes. - sock->Close(); + dap->RequestTermination(); } } @@ -542,7 +541,7 @@ int main(int argc, char *argv[]) { lldb::IOObjectSP output = std::make_shared( stdout_fd, File::eOpenOptionWriteOnly, NativeFile::Unowned); - constexpr llvm::StringLiteral client_name = "stdin/stdout"; + constexpr llvm::StringLiteral client_name = "stdio"; Transport transport(client_name, log.get(), input, output); DAP dap(log.get(), default_repl_mode, pre_init_commands, transport); @@ -557,7 +556,7 @@ int main(int argc, char *argv[]) { if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr) redirection_test(); - if (auto Err = dap.Loop()) { + if (auto Err = dap.Run()) { DAP_LOG(log.get(), "({0}) DAP session error: {1}", client_name, llvm::toStringWithoutConsuming(Err)); llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),