https://github.com/qxy11 updated https://github.com/llvm/llvm-project/pull/163653
>From 5b94fa5412e8a56ee1ea1fe7e1cd8d9efddcaffb Mon Sep 17 00:00:00 2001 From: qxy11 <[email protected]> Date: Wed, 15 Oct 2025 15:47:23 -0700 Subject: [PATCH 1/3] [lldb-dap] Add multi-session support with shared debugger instances Summary: This change introduces a DAPSessionManager to enable multiple concurrent lldb-dap sessions and allow them to share debugger instances when needed. Key changes: - Add DAPSessionManager singleton to track and coordinate all active DAP sessions - Support attaching to an existing target via unique target ID (targetId parameter) - Share debugger instances across sessions when spawning new targets (e.g., GPU debugging) - Refactor event thread management to allow sharing event threads between sessions - Add eBroadcastBitNewTargetCreated event to notify when new targets are spawned - Extract session names from target creation events for better identification - Defer debugger initialization from 'initialize' request to 'launch'/'attach' requests This enables scenarios where a native process spawns a new target (like a child process) and the debug adapter can automatically start a new debug session for the spawned target while sharing the parent's debugger instance. Tests: The refactoring maintains backward compatibility. All existing test cases pass. --- lldb/include/lldb/API/SBTarget.h | 3 + lldb/include/lldb/Target/Target.h | 9 + .../test/tools/lldb-dap/dap_server.py | 3 + lldb/source/API/SBTarget.cpp | 8 + lldb/source/Target/Target.cpp | 34 ++- .../tools/lldb-dap/attach/TestDAP_attach.py | 13 + lldb/tools/lldb-dap/CMakeLists.txt | 1 + lldb/tools/lldb-dap/DAP.cpp | 231 ++++++++++++++---- lldb/tools/lldb-dap/DAP.h | 25 +- lldb/tools/lldb-dap/DAPSessionManager.cpp | 168 +++++++++++++ lldb/tools/lldb-dap/DAPSessionManager.h | 119 +++++++++ .../lldb-dap/Handler/AttachRequestHandler.cpp | 26 +- .../Handler/InitializeRequestHandler.cpp | 58 +---- .../lldb-dap/Handler/LaunchRequestHandler.cpp | 4 + .../lldb-dap/Protocol/ProtocolRequests.cpp | 3 +- .../lldb-dap/Protocol/ProtocolRequests.h | 3 + lldb/tools/lldb-dap/package.json | 4 + lldb/tools/lldb-dap/tool/lldb-dap.cpp | 53 ++-- lldb/unittests/DAP/CMakeLists.txt | 1 + lldb/unittests/DAP/DAPSessionManagerTest.cpp | 177 ++++++++++++++ .../gn/secondary/lldb/tools/lldb-dap/BUILD.gn | 1 + 21 files changed, 802 insertions(+), 142 deletions(-) create mode 100644 lldb/tools/lldb-dap/DAPSessionManager.cpp create mode 100644 lldb/tools/lldb-dap/DAPSessionManager.h create mode 100644 lldb/unittests/DAP/DAPSessionManagerTest.cpp diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 173fd05b54a13..1d251763a826a 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -44,6 +44,7 @@ class LLDB_API SBTarget { eBroadcastBitWatchpointChanged = (1 << 3), eBroadcastBitSymbolsLoaded = (1 << 4), eBroadcastBitSymbolsChanged = (1 << 5), + eBroadcastBitNewTargetCreated = (1 << 6), }; // Constructors @@ -69,6 +70,8 @@ class LLDB_API SBTarget { static lldb::SBModule GetModuleAtIndexFromEvent(const uint32_t idx, const lldb::SBEvent &event); + static const char *GetSessionNameFromEvent(const SBEvent &event); + static const char *GetBroadcasterClassName(); lldb::SBProcess GetProcess(); diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index f4a09237ce897..86349897762f4 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -537,6 +537,7 @@ class Target : public std::enable_shared_from_this<Target>, eBroadcastBitWatchpointChanged = (1 << 3), eBroadcastBitSymbolsLoaded = (1 << 4), eBroadcastBitSymbolsChanged = (1 << 5), + eBroadcastBitNewTargetCreated = (1 << 6), }; // These two functions fill out the Broadcaster interface: @@ -556,6 +557,11 @@ class Target : public std::enable_shared_from_this<Target>, TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list); + TargetEventData(const lldb::TargetSP &target_sp, std::string session_name); + + TargetEventData(const lldb::TargetSP &target_sp, + const ModuleList &module_list, std::string session_name); + ~TargetEventData() override; static llvm::StringRef GetFlavorString(); @@ -564,6 +570,8 @@ class Target : public std::enable_shared_from_this<Target>, return TargetEventData::GetFlavorString(); } + static llvm::StringRef GetSessionNameFromEvent(const Event *event_ptr); + void Dump(Stream *s) const override; static const TargetEventData *GetEventDataFromEvent(const Event *event_ptr); @@ -579,6 +587,7 @@ class Target : public std::enable_shared_from_this<Target>, private: lldb::TargetSP m_target_sp; ModuleList m_module_list; + std::string m_session_name; TargetEventData(const TargetEventData &) = delete; const TargetEventData &operator=(const TargetEventData &) = delete; 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 8eb64b4ab8b2b..b52aaf558aa24 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 @@ -785,6 +785,7 @@ def request_attach( *, program: Optional[str] = None, pid: Optional[int] = None, + targetId: Optional[int] = None, waitFor=False, initCommands: Optional[list[str]] = None, preRunCommands: Optional[list[str]] = None, @@ -804,6 +805,8 @@ def request_attach( args_dict["pid"] = pid if program is not None: args_dict["program"] = program + if targetId is not None: + args_dict["targetId"] = targetId if waitFor: args_dict["waitFor"] = waitFor args_dict["initCommands"] = self.init_commands diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 98d10aa07c53f..0f4508c772ee4 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -145,6 +145,14 @@ SBModule SBTarget::GetModuleAtIndexFromEvent(const uint32_t idx, return SBModule(module_list.GetModuleAtIndex(idx)); } +const char *SBTarget::GetSessionNameFromEvent(const SBEvent &event) { + LLDB_INSTRUMENT_VA(event); + + return ConstString( + Target::TargetEventData::GetSessionNameFromEvent(event.get())) + .AsCString(); +} + const char *SBTarget::GetBroadcasterClassName() { LLDB_INSTRUMENT(); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index e224a12e33463..a13e11c4b3a05 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -193,6 +193,7 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, SetEventName(eBroadcastBitModulesUnloaded, "modules-unloaded"); SetEventName(eBroadcastBitWatchpointChanged, "watchpoint-changed"); SetEventName(eBroadcastBitSymbolsLoaded, "symbols-loaded"); + SetEventName(eBroadcastBitNewTargetCreated, "new-target-created"); CheckInWithManager(); @@ -5169,11 +5170,21 @@ void TargetProperties::SetDebugUtilityExpression(bool debug) { // Target::TargetEventData Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp) - : EventData(), m_target_sp(target_sp), m_module_list() {} + : TargetEventData(target_sp, ModuleList(), "") {} Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list) - : EventData(), m_target_sp(target_sp), m_module_list(module_list) {} + : TargetEventData(target_sp, module_list, "") {} + +Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, + std::string session_name) + : TargetEventData(target_sp, ModuleList(), std::move(session_name)) {} + +Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, + const ModuleList &module_list, + std::string session_name) + : EventData(), m_target_sp(target_sp), m_module_list(module_list), + m_session_name(std::move(session_name)) {} Target::TargetEventData::~TargetEventData() = default; @@ -5209,6 +5220,25 @@ TargetSP Target::TargetEventData::GetTargetFromEvent(const Event *event_ptr) { return target_sp; } +llvm::StringRef +Target::TargetEventData::GetSessionNameFromEvent(const Event *event_ptr) { + const TargetEventData *event_data = GetEventDataFromEvent(event_ptr); + if (!event_data) + return llvm::StringRef(); + + if (!event_data->m_session_name.empty()) + return event_data->m_session_name; + + // Generate default session name if not provided + if (event_data->m_target_sp) { + lldb::user_id_t target_id = event_data->m_target_sp->GetGloballyUniqueID(); + std::string default_name = llvm::formatv("Session {0}", target_id).str(); + return ConstString(default_name).GetStringRef(); + } + + return llvm::StringRef(); +} + ModuleList Target::TargetEventData::GetModuleListFromEvent(const Event *event_ptr) { ModuleList module_list; 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 5331a9f94ef1f..6ca286c12abd2 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -72,3 +72,16 @@ def test_by_name_waitFor(self): self.spawn_thread.start() self.attach(program=program, waitFor=True) self.continue_and_verify_pid() + + def test_attach_with_invalid_targetId(self): + """ + Test that attaching with an invalid targetId fails with the expected + error message. + """ + self.build_and_create_debug_adapter() + + resp = self.attach(targetId=99999, expectFailure=True) + self.assertFalse(resp["success"]) + self.assertIn( + "Unable to find existing debugger", resp["body"]["error"]["format"] + ) diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 7db334ca56bcf..2fbdc73a79978 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -12,6 +12,7 @@ add_lldb_library(lldbDAP DAP.cpp DAPError.cpp DAPLog.cpp + DAPSessionManager.cpp EventHelper.cpp ExceptionBreakpoint.cpp FifoFiles.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index f76656e98ca01..dc681336637e0 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "DAP.h" +#include "CommandPlugins.h" #include "DAPLog.h" #include "EventHelper.h" #include "ExceptionBreakpoint.h" @@ -241,10 +242,12 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { } void DAP::StopEventHandlers() { - if (event_thread.joinable()) { - broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); - event_thread.join(); - } + event_thread_sp.reset(); + + // Clean up expired event threads from the session manager. + DAPSessionManager::GetInstance().ReleaseExpiredEventThreads(); + + // Still handle the progress thread normally since it's per-DAP instance. if (progress_event_thread.joinable()) { broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread); progress_event_thread.join(); @@ -804,7 +807,8 @@ void DAP::SetTarget(const lldb::SBTarget target) { lldb::SBTarget::eBroadcastBitModulesLoaded | lldb::SBTarget::eBroadcastBitModulesUnloaded | lldb::SBTarget::eBroadcastBitSymbolsLoaded | - lldb::SBTarget::eBroadcastBitSymbolsChanged); + lldb::SBTarget::eBroadcastBitSymbolsChanged | + lldb::SBTarget::eBroadcastBitNewTargetCreated); listener.StartListeningForEvents(this->broadcaster, eBroadcastBitStopEventThread); } @@ -1289,13 +1293,91 @@ protocol::Capabilities DAP::GetCustomCapabilities() { } void DAP::StartEventThread() { - event_thread = std::thread(&DAP::EventThread, this); + // Get event thread for this debugger (creates it if it doesn't exist). + event_thread_sp = DAPSessionManager::GetInstance().GetEventThreadForDebugger( + debugger, this); } void DAP::StartProgressEventThread() { progress_event_thread = std::thread(&DAP::ProgressEventThread, this); } +void DAP::StartEventThreads() { + if (clientFeatures.contains(eClientFeatureProgressReporting)) + StartProgressEventThread(); + + StartEventThread(); +} + +llvm::Error DAP::InitializeDebugger(std::optional<uint32_t> target_id) { + // Initialize debugger instance (shared or individual). + if (target_id) { + std::optional<lldb::SBDebugger> shared_debugger = + DAPSessionManager::GetInstance().GetSharedDebugger(*target_id); + // If the target ID is not valid, then we won't find a debugger. + if (!shared_debugger) { + return llvm::createStringError( + "Unable to find existing debugger for target ID"); + } + debugger = shared_debugger.value(); + StartEventThreads(); + return llvm::Error::success(); + } + + debugger = lldb::SBDebugger::Create(/*argument_name=*/false); + + // Configure input/output/error file descriptors. + debugger.SetInputFile(in); + target = debugger.GetDummyTarget(); + + llvm::Expected<int> out_fd = out.GetWriteFileDescriptor(); + if (!out_fd) + return out_fd.takeError(); + debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false)); + + llvm::Expected<int> err_fd = err.GetWriteFileDescriptor(); + if (!err_fd) + return err_fd.takeError(); + debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false)); + + // The sourceInitFile option is not part of the DAP specification. It is an + // extension used by the test suite to prevent sourcing `.lldbinit` and + // changing its behavior. The CLI flag --no-lldbinit takes precedence over + // the DAP parameter. + bool should_source_init_files = !no_lldbinit && sourceInitFile; + if (should_source_init_files) { + debugger.SkipLLDBInitFiles(false); + debugger.SkipAppInitFiles(false); + lldb::SBCommandReturnObject init; + auto interp = debugger.GetCommandInterpreter(); + interp.SourceInitFileInGlobalDirectory(init); + interp.SourceInitFileInHomeDirectory(init); + } + + // Run initialization commands. + if (llvm::Error err = RunPreInitCommands()) + return err; + + auto cmd = debugger.GetCommandInterpreter().AddMultiwordCommand( + "lldb-dap", "Commands for managing lldb-dap."); + + if (clientFeatures.contains(eClientFeatureStartDebuggingRequest)) { + cmd.AddCommand( + "start-debugging", new StartDebuggingCommand(*this), + "Sends a startDebugging request from the debug adapter to the client " + "to start a child debug session of the same type as the caller."); + } + + cmd.AddCommand( + "repl-mode", new ReplModeCommand(*this), + "Get or set the repl behavior of lldb-dap evaluation requests."); + cmd.AddCommand("send-event", new SendEventCommand(*this), + "Sends an DAP event to the client."); + + StartEventThreads(); + return llvm::Error::success(); +} + void DAP::ProgressEventThread() { lldb::SBListener listener("lldb-dap.progress.listener"); debugger.GetBroadcaster().AddListener( @@ -1374,6 +1456,14 @@ void DAP::EventThread() { const auto event_mask = event.GetType(); if (lldb::SBProcess::EventIsProcessEvent(event)) { lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event); + // Find the DAP instance that owns this process's target. + DAP *dap_instance = DAPSessionManager::FindDAP(process.GetTarget()); + if (!dap_instance) { + DAP_LOG(log, "Unable to find DAP instance for process {0}", + process.GetProcessID()); + continue; + } + if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { auto state = lldb::SBProcess::GetStateFromEvent(event); switch (state) { @@ -1390,89 +1480,138 @@ void DAP::EventThread() { // Only report a stopped event if the process was not // automatically restarted. if (!lldb::SBProcess::GetRestartedFromEvent(event)) { - SendStdOutStdErr(*this, process); - if (llvm::Error err = SendThreadStoppedEvent(*this)) - DAP_LOG_ERROR(log, std::move(err), + SendStdOutStdErr(*dap_instance, process); + if (llvm::Error err = SendThreadStoppedEvent(*dap_instance)) + DAP_LOG_ERROR(dap_instance->log, std::move(err), "({1}) reporting thread stopped: {0}", - m_client_name); + dap_instance->m_client_name); } break; case lldb::eStateRunning: case lldb::eStateStepping: - WillContinue(); - SendContinuedEvent(*this); + dap_instance->WillContinue(); + SendContinuedEvent(*dap_instance); break; case lldb::eStateExited: lldb::SBStream stream; process.GetStatus(stream); - SendOutput(OutputType::Console, stream.GetData()); + dap_instance->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; + process.GetProcessID() == dap_instance->restarting_process_id) { + dap_instance->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(); + dap_instance->RunExitCommands(); + SendProcessExitedEvent(*dap_instance, process); + dap_instance->SendTerminatedEvent(); done = true; } break; } } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) || (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) { - SendStdOutStdErr(*this, process); + SendStdOutStdErr(*dap_instance, 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) { + lldb::SBTarget event_target = + lldb::SBTarget::GetTargetFromEvent(event); + // Find the DAP instance that owns this target. + DAP *dap_instance = DAPSessionManager::FindDAP(event_target); + if (!dap_instance) + continue; + const uint32_t num_modules = lldb::SBTarget::GetNumModulesFromEvent(event); const bool remove_module = event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded; - std::lock_guard<std::mutex> guard(modules_mutex); + std::lock_guard<std::mutex> guard(dap_instance->modules_mutex); for (uint32_t i = 0; i < num_modules; ++i) { lldb::SBModule module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event); std::optional<protocol::Module> p_module = - CreateModule(target, module, remove_module); + CreateModule(dap_instance->target, module, remove_module); if (!p_module) continue; llvm::StringRef module_id = p_module->id; - const bool module_exists = modules.contains(module_id); + const bool module_exists = + dap_instance->modules.contains(module_id); if (remove_module && module_exists) { - modules.erase(module_id); - Send(protocol::Event{ + dap_instance->modules.erase(module_id); + dap_instance->Send(protocol::Event{ "module", ModuleEventBody{std::move(p_module).value(), ModuleEventBody::eReasonRemoved}}); } else if (module_exists) { - Send(protocol::Event{ + dap_instance->Send(protocol::Event{ "module", ModuleEventBody{std::move(p_module).value(), ModuleEventBody::eReasonChanged}}); } else if (!remove_module) { - modules.insert(module_id); - Send(protocol::Event{ + dap_instance->modules.insert(module_id); + dap_instance->Send(protocol::Event{ "module", ModuleEventBody{std::move(p_module).value(), ModuleEventBody::eReasonNew}}); } } + } else if (event_mask & lldb::SBTarget::eBroadcastBitNewTargetCreated) { + auto target = lldb::SBTarget::GetTargetFromEvent(event); + + // Generate unique target ID and set the shared debugger. + uint32_t target_id = target.GetGloballyUniqueID(); + DAPSessionManager::GetInstance().SetSharedDebugger(target_id, + debugger); + + // We create an attach config that will select the unique + // target ID of the created target. The DAP instance will attach to + // this existing target and the debug session will be ready to go. + llvm::json::Object attach_config; + + // If we have a process name, add command to attach to the same + // process name. + attach_config.try_emplace("type", "lldb"); + attach_config.try_emplace("targetId", target_id); + const char *session_name = + lldb::SBTarget::GetSessionNameFromEvent(event); + attach_config.try_emplace("name", session_name); + + // 2. Construct the main 'startDebugging' request arguments. + llvm::json::Object start_debugging_args{ + {"request", "attach"}, + {"configuration", std::move(attach_config)}}; + + // Send the request. Note that this is a reverse request, so you don't + // expect a direct response in the same way as a client request. + SendReverseRequest<LogFailureResponseHandler>( + "startDebugging", std::move(start_debugging_args)); } } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) { + lldb::SBBreakpoint bp = + lldb::SBBreakpoint::GetBreakpointFromEvent(event); + if (!bp.IsValid()) + continue; + + lldb::SBTarget event_target = bp.GetTarget(); + + // Find the DAP instance that owns this target. + DAP *dap_instance = DAPSessionManager::FindDAP(event_target); + if (!dap_instance) + continue; + if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) { auto event_type = lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); - auto bp = Breakpoint( - *this, lldb::SBBreakpoint::GetBreakpointFromEvent(event)); + auto breakpoint = Breakpoint(*dap_instance, bp); // 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 @@ -1480,14 +1619,14 @@ void DAP::EventThread() { if ((event_type & lldb::eBreakpointEventTypeLocationsAdded || event_type & lldb::eBreakpointEventTypeLocationsRemoved || event_type & lldb::eBreakpointEventTypeLocationsResolved) && - bp.MatchesName(BreakpointBase::kDAPBreakpointLabel)) { + breakpoint.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. - protocol::Breakpoint protocol_bp = bp.ToProtocolBreakpoint(); - + protocol::Breakpoint protocol_bp = + breakpoint.ToProtocolBreakpoint(); // "source" is not needed here, unless we add adapter data to be // saved by the client. if (protocol_bp.source && !protocol_bp.source->adapterData) @@ -1500,19 +1639,29 @@ void DAP::EventThread() { llvm::json::Object bp_event = CreateEventObject("breakpoint"); bp_event.try_emplace("body", std::move(body)); - SendJSON(llvm::json::Value(std::move(bp_event))); + dap_instance->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()) - 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()); + // Global debugger events - send to all DAP instances. + std::vector<DAP *> active_instances = + DAPSessionManager::GetInstance().GetActiveSessions(); + for (DAP *dap_instance : active_instances) { + if (!dap_instance) + continue; + + lldb::SBStructuredData data = + lldb::SBDebugger::GetDiagnosticFromEvent(event); + if (!data.IsValid()) + continue; + + std::string type = GetStringValue(data.GetValueForKey("type")); + std::string message = GetStringValue(data.GetValueForKey("message")); + dap_instance->SendOutput( + OutputType::Important, + llvm::formatv("{0}: {1}", type, message).str()); + } } else if (event.BroadcasterMatchesRef(broadcaster)) { if (event_mask & eBroadcastBitStopEventThread) { done = true; diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index a90ddf59671ee..fdca801d400c0 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -10,6 +10,7 @@ #define LLDB_TOOLS_LLDB_DAP_DAP_H #include "DAPForward.h" +#include "DAPSessionManager.h" #include "ExceptionBreakpoint.h" #include "FunctionBreakpoint.h" #include "InstructionBreakpoint.h" @@ -47,6 +48,7 @@ #include <condition_variable> #include <cstdint> #include <deque> +#include <map> #include <memory> #include <mutex> #include <optional> @@ -81,6 +83,8 @@ enum class ReplMode { Variable = 0, Command, Auto }; using DAPTransport = lldb_private::transport::JSONTransport<ProtocolDescriptor>; struct DAP final : public DAPTransport::MessageHandler { + friend class DAPSessionManager; + /// Path to the lldb-dap binary itself. static llvm::StringRef debug_adapter_path; @@ -157,6 +161,11 @@ struct DAP final : public DAPTransport::MessageHandler { /// Whether to disable sourcing .lldbinit files. bool no_lldbinit; + /// Stores whether the initialize request specified a value for + /// lldbExtSourceInitFile. Used by the test suite to prevent sourcing + /// `.lldbinit` and changing its behavior. + bool sourceInitFile = true; + /// The initial thread list upon attaching. std::vector<protocol::Thread> initial_thread_list; @@ -417,6 +426,18 @@ struct DAP final : public DAPTransport::MessageHandler { void StartEventThread(); void StartProgressEventThread(); + /// DAP debugger initialization functions. + /// @{ + + /// Perform complete DAP initialization in one call. + llvm::Error + InitializeDebugger(std::optional<uint32_t> target_idx = std::nullopt); + + /// Start event handling threads based on client capabilities. + void StartEventThreads(); + + /// @} + /// Sets the given protocol `breakpoints` in the given `source`, while /// removing any existing breakpoints in the given source if they are not in /// `breakpoint`. @@ -462,7 +483,9 @@ struct DAP final : public DAPTransport::MessageHandler { void EventThread(); void ProgressEventThread(); - std::thread event_thread; + /// Event thread is a shared pointer in case we have a multiple + /// DAP instances sharing the same event thread. + std::shared_ptr<ManagedEventThread> event_thread_sp; std::thread progress_event_thread; /// @} diff --git a/lldb/tools/lldb-dap/DAPSessionManager.cpp b/lldb/tools/lldb-dap/DAPSessionManager.cpp new file mode 100644 index 0000000000000..5fd6e7e810fcc --- /dev/null +++ b/lldb/tools/lldb-dap/DAPSessionManager.cpp @@ -0,0 +1,168 @@ +//===-- DAPSessionManager.cpp ----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +#include "DAPSessionManager.h" +#include "DAP.h" +#include "lldb/API/SBBroadcaster.h" +#include "lldb/API/SBEvent.h" +#include "lldb/API/SBTarget.h" +#include "lldb/Host/MainLoopBase.h" +#include "llvm/Support/Threading.h" +#include "llvm/Support/WithColor.h" + +#include <chrono> +#include <mutex> + +namespace lldb_dap { + +ManagedEventThread::ManagedEventThread(lldb::SBBroadcaster broadcaster, + std::thread t) + : m_broadcaster(broadcaster), m_event_thread(std::move(t)) {} + +ManagedEventThread::~ManagedEventThread() { + if (m_event_thread.joinable()) { + m_broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); + m_event_thread.join(); + } +} + +DAPSessionManager &DAPSessionManager::GetInstance() { + // NOTE: intentional leak to avoid issues with C++ destructor chain + // Use std::call_once for thread-safe initialization. + static std::once_flag initialized; + static DAPSessionManager *instance = nullptr; + + std::call_once(initialized, []() { instance = new DAPSessionManager(); }); + + return *instance; +} + +void DAPSessionManager::RegisterSession(lldb_private::MainLoop *loop, + DAP *dap) { + std::lock_guard<std::mutex> lock(m_sessions_mutex); + m_active_sessions[loop] = dap; +} + +void DAPSessionManager::UnregisterSession(lldb_private::MainLoop *loop) { + std::unique_lock<std::mutex> lock(m_sessions_mutex); + m_active_sessions.erase(loop); + + // Clean up shared resources when the last session exits. + if (m_active_sessions.empty()) + CleanupSharedResources(); + + std::notify_all_at_thread_exit(m_sessions_condition, std::move(lock)); +} + +std::vector<DAP *> DAPSessionManager::GetActiveSessions() { + std::lock_guard<std::mutex> lock(m_sessions_mutex); + std::vector<DAP *> sessions; + for (const auto &[loop, dap] : m_active_sessions) + if (dap) + sessions.emplace_back(dap); + return sessions; +} + +void DAPSessionManager::DisconnectAllSessions() { + std::lock_guard<std::mutex> lock(m_sessions_mutex); + m_client_failed = false; + for (auto [loop, dap] : m_active_sessions) { + if (dap) { + if (llvm::Error error = dap->Disconnect()) { + m_client_failed = true; + llvm::WithColor::error() << "DAP client disconnected failed: " + << llvm::toString(std::move(error)) << "\n"; + } + loop->AddPendingCallback( + [](lldb_private::MainLoopBase &loop) { loop.RequestTermination(); }); + } + } +} + +llvm::Error DAPSessionManager::WaitForAllSessionsToDisconnect() { + std::unique_lock<std::mutex> lock(m_sessions_mutex); + m_sessions_condition.wait(lock, [this] { return m_active_sessions.empty(); }); + + // Check if any disconnection failed and return appropriate error. + if (m_client_failed) + return llvm::make_error<llvm::StringError>( + "disconnecting all clients failed", llvm::inconvertibleErrorCode()); + + return llvm::Error::success(); +} + +std::shared_ptr<ManagedEventThread> +DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, + DAP *requesting_dap) { + lldb::user_id_t debugger_id = debugger.GetID(); + std::lock_guard<std::mutex> lock(m_sessions_mutex); + + // Try to use shared event thread, if it exists. + if (auto it = m_debugger_event_threads.find(debugger_id); + it != m_debugger_event_threads.end()) { + if (auto thread_sp = it->second.lock()) + return thread_sp; + // Our weak pointer has expired. + m_debugger_event_threads.erase(it); + } + + // Create a new event thread and store it. + auto new_thread_sp = std::make_shared<ManagedEventThread>( + requesting_dap->broadcaster, + std::thread(&DAP::EventThread, requesting_dap)); + m_debugger_event_threads[debugger_id] = new_thread_sp; + return new_thread_sp; +} + +void DAPSessionManager::SetSharedDebugger(uint32_t target_id, + lldb::SBDebugger debugger) { + std::lock_guard<std::mutex> lock(m_sessions_mutex); + m_target_to_debugger_map[target_id] = debugger; +} + +std::optional<lldb::SBDebugger> +DAPSessionManager::GetSharedDebugger(uint32_t target_id) { + std::lock_guard<std::mutex> lock(m_sessions_mutex); + auto pos = m_target_to_debugger_map.find(target_id); + if (pos == m_target_to_debugger_map.end()) + return std::nullopt; + lldb::SBDebugger debugger = pos->second; + m_target_to_debugger_map.erase(pos); + return debugger; +} + +DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { + std::lock_guard<std::mutex> lock(m_sessions_mutex); + + for (const auto &[loop, dap] : m_active_sessions) + if (dap && dap->target.IsValid() && dap->target == target) + return dap; + + return nullptr; +} + +void DAPSessionManager::CleanupSharedResources() { + // SBDebugger destructors will handle cleanup when the map entries are + // destroyed. + m_target_to_debugger_map.clear(); +} + +void DAPSessionManager::ReleaseExpiredEventThreads() { + std::lock_guard<std::mutex> lock(m_sessions_mutex); + for (auto it = m_debugger_event_threads.begin(); + it != m_debugger_event_threads.end();) { + // Check if the weak_ptr has expired (no DAP instances are using it + // anymore). + if (it->second.expired()) { + it = m_debugger_event_threads.erase(it); + } else { + ++it; + } + } +} + +} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/DAPSessionManager.h b/lldb/tools/lldb-dap/DAPSessionManager.h new file mode 100644 index 0000000000000..d547f3036a9b2 --- /dev/null +++ b/lldb/tools/lldb-dap/DAPSessionManager.h @@ -0,0 +1,119 @@ +//===-- DAPSessionManager.h ------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H +#define LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H + +#include "lldb/API/SBBroadcaster.h" +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBTarget.h" +#include "lldb/Host/MainLoop.h" +#include "lldb/lldb-types.h" +#include "llvm/Support/Error.h" +#include <condition_variable> +#include <map> +#include <memory> +#include <mutex> +#include <optional> +#include <thread> +#include <vector> + +namespace lldb_dap { + +// Forward declarations +struct DAP; + +class ManagedEventThread { +public: + // Constructor declaration + ManagedEventThread(lldb::SBBroadcaster broadcaster, std::thread t); + + ~ManagedEventThread(); + + ManagedEventThread(const ManagedEventThread &) = delete; + ManagedEventThread &operator=(const ManagedEventThread &) = delete; + +private: + lldb::SBBroadcaster m_broadcaster; + std::thread m_event_thread; +}; + +/// Global DAP session manager. +class DAPSessionManager { +public: + /// Get the singleton instance of the DAP session manager. + static DAPSessionManager &GetInstance(); + + /// Register a DAP session. + void RegisterSession(lldb_private::MainLoop *loop, DAP *dap); + + /// Unregister a DAP session. + void UnregisterSession(lldb_private::MainLoop *loop); + + /// Get all active DAP sessions. + std::vector<DAP *> GetActiveSessions(); + + /// Disconnect all active sessions. + void DisconnectAllSessions(); + + /// Wait for all sessions to finish disconnecting. + /// Returns an error if any client disconnection failed, otherwise success. + llvm::Error WaitForAllSessionsToDisconnect(); + + /// Set the shared debugger instance for a unique target ID. + void SetSharedDebugger(uint32_t target_id, lldb::SBDebugger debugger); + + /// Get the shared debugger instance for a unique target ID. + std::optional<lldb::SBDebugger> GetSharedDebugger(uint32_t target_id); + + /// Get or create event thread for a specific debugger. + std::shared_ptr<ManagedEventThread> + GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP *requesting_dap); + + /// Find the DAP instance that owns the given target. + DAP *FindDAPForTarget(lldb::SBTarget target); + + /// Static convenience method for FindDAPForTarget. + static DAP *FindDAP(lldb::SBTarget target) { + return GetInstance().FindDAPForTarget(target); + } + + /// Clean up shared resources when the last session exits. + void CleanupSharedResources(); + + /// Clean up expired event threads from the collection. + void ReleaseExpiredEventThreads(); + +private: + DAPSessionManager() = default; + ~DAPSessionManager() = default; + + // Non-copyable and non-movable. + DAPSessionManager(const DAPSessionManager &) = delete; + DAPSessionManager &operator=(const DAPSessionManager &) = delete; + DAPSessionManager(DAPSessionManager &&) = delete; + DAPSessionManager &operator=(DAPSessionManager &&) = delete; + + bool m_client_failed = false; + std::mutex m_sessions_mutex; + std::condition_variable m_sessions_condition; + std::map<lldb_private::MainLoop *, DAP *> m_active_sessions; + + /// Optional map from target ID to shared debugger set when the native + /// process creates a new target. + std::map<uint32_t, lldb::SBDebugger> m_target_to_debugger_map; + + /// Map from debugger ID to its event thread used for when + /// multiple DAP sessions are using the same debugger instance. + std::map<lldb::user_id_t, std::weak_ptr<ManagedEventThread>> + m_debugger_event_threads; +}; + +} // namespace lldb_dap + +#endif // LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 371349a26866e..cd30fc5b6325f 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -17,6 +17,7 @@ #include "lldb/lldb-defines.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" +#include <cstdint> using namespace llvm; using namespace lldb_dap::protocol; @@ -29,14 +30,20 @@ namespace lldb_dap { /// Since attaching is debugger/runtime specific, the arguments for this request /// are not part of this specification. Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { + // Initialize DAP debugger and related components if not sharing previously + // launched debugger. + std::optional<uint32_t> target_id = args.targetId; + if (Error err = dap.InitializeDebugger(target_id)) + return err; + // Validate that we have a well formed attach request. if (args.attachCommands.empty() && args.coreFile.empty() && args.configuration.program.empty() && args.pid == LLDB_INVALID_PROCESS_ID && - args.gdbRemotePort == LLDB_DAP_INVALID_PORT) + args.gdbRemotePort == LLDB_DAP_INVALID_PORT && !target_id.has_value()) return make_error<DAPError>( "expected one of 'pid', 'program', 'attachCommands', " - "'coreFile' or 'gdb-remote-port' to be specified"); + "'coreFile', 'gdb-remote-port', or target_id to be specified"); // Check if we have mutually exclusive arguments. if ((args.pid != LLDB_INVALID_PROCESS_ID) && @@ -64,7 +71,18 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { dap.ConfigureSourceMaps(); lldb::SBError error; - lldb::SBTarget target = dap.CreateTarget(error); + lldb::SBTarget target; + if (target_id) { + // Use the unique target ID to get the target. + target = dap.debugger.FindTargetByGloballyUniqueID(*target_id); + if (!target.IsValid()) { + error.SetErrorStringWithFormat("invalid target_id %u in attach config", + *target_id); + } + } else { + target = dap.CreateTarget(error); + } + if (error.Fail()) return ToError(error); @@ -114,7 +132,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { connect_url += std::to_string(args.gdbRemotePort); dap.target.ConnectRemote(listener, connect_url.c_str(), "gdb-remote", error); - } else { + } else if (!target_id.has_value()) { // Attach by pid or process name. lldb::SBAttachInfo attach_info; if (args.pid != LLDB_INVALID_PROCESS_ID) diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index 9069de4a3a690..53e1810a5b0e0 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -21,63 +21,9 @@ using namespace lldb_dap::protocol; /// Initialize request; value of command field is 'initialize'. llvm::Expected<InitializeResponse> InitializeRequestHandler::Run( const InitializeRequestArguments &arguments) const { + // Store initialization arguments for later use in Launch/Attach. dap.clientFeatures = arguments.supportedFeatures; - - // Do not source init files until in/out/err are configured. - dap.debugger = lldb::SBDebugger::Create(false); - dap.debugger.SetInputFile(dap.in); - dap.target = dap.debugger.GetDummyTarget(); - - llvm::Expected<int> out_fd = dap.out.GetWriteFileDescriptor(); - if (!out_fd) - return out_fd.takeError(); - dap.debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false)); - - llvm::Expected<int> err_fd = dap.err.GetWriteFileDescriptor(); - if (!err_fd) - return err_fd.takeError(); - dap.debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false)); - - auto interp = dap.debugger.GetCommandInterpreter(); - - // The sourceInitFile option is not part of the DAP specification. It is an - // extension used by the test suite to prevent sourcing `.lldbinit` and - // changing its behavior. The CLI flag --no-lldbinit takes precedence over - // the DAP parameter. - bool should_source_init_files = - !dap.no_lldbinit && arguments.lldbExtSourceInitFile.value_or(true); - if (should_source_init_files) { - dap.debugger.SkipLLDBInitFiles(false); - dap.debugger.SkipAppInitFiles(false); - lldb::SBCommandReturnObject init; - interp.SourceInitFileInGlobalDirectory(init); - interp.SourceInitFileInHomeDirectory(init); - } - - if (llvm::Error err = dap.RunPreInitCommands()) - return err; - - auto cmd = dap.debugger.GetCommandInterpreter().AddMultiwordCommand( - "lldb-dap", "Commands for managing lldb-dap."); - if (arguments.supportedFeatures.contains( - eClientFeatureStartDebuggingRequest)) { - cmd.AddCommand( - "start-debugging", new StartDebuggingCommand(dap), - "Sends a startDebugging request from the debug adapter to the client " - "to start a child debug session of the same type as the caller."); - } - cmd.AddCommand( - "repl-mode", new ReplModeCommand(dap), - "Get or set the repl behavior of lldb-dap evaluation requests."); - cmd.AddCommand("send-event", new SendEventCommand(dap), - "Sends an DAP event to the client."); - - if (arguments.supportedFeatures.contains(eClientFeatureProgressReporting)) - dap.StartProgressEventThread(); - - // Start our event thread so we can receive events from the debugger, target, - // process and more. - dap.StartEventThread(); + dap.sourceInitFile = arguments.lldbExtSourceInitFile.value_or(true); return dap.GetCapabilities(); } diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 553cbeaf849e2..329f0a7bf6453 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -22,6 +22,10 @@ namespace lldb_dap { /// Launch request; value of command field is 'launch'. Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { + // Initialize DAP debugger. + if (Error err = dap.InitializeDebugger()) + return err; + // Validate that we have a well formed launch request. if (!arguments.launchCommands.empty() && arguments.console != protocol::eConsoleInternal) diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index b9393356b4e01..77a23df411428 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -316,7 +316,8 @@ bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA, O.mapOptional("waitFor", ARA.waitFor) && O.mapOptional("gdb-remote-port", ARA.gdbRemotePort) && O.mapOptional("gdb-remote-hostname", ARA.gdbRemoteHostname) && - O.mapOptional("coreFile", ARA.coreFile); + O.mapOptional("coreFile", ARA.coreFile) && + O.mapOptional("targetId", ARA.targetId); } bool fromJSON(const json::Value &Params, ContinueArguments &CA, json::Path P) { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 92dada2295841..a762071cb61c5 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -349,6 +349,9 @@ struct AttachRequestArguments { /// Path to the core file to debug. std::string coreFile; + /// Unique ID of an existing target to attach to. + std::optional<uint32_t> targetId; + /// @} }; bool fromJSON(const llvm::json::Value &, AttachRequestArguments &, diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index e961c2e48b258..b008a24137216 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -775,6 +775,10 @@ "description": "Custom commands that are executed instead of attaching to a process ID or to a process by name. These commands may optionally create a new target and must perform an attach. A valid process must exist after these commands complete or the \"attach\" will fail.", "default": [] }, + "targetId": { + "type": "number", + "description": "The globally unique target id to attach to. Used when a target is dynamically created." + }, "initCommands": { "type": "array", "items": { diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index 93446c051eb54..eeb3e45e07b32 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -320,12 +320,8 @@ static llvm::Error serveConnection( g_connection_timeout_time_point, connection_timeout_seconds.value()); std::condition_variable dap_sessions_condition; - std::mutex dap_sessions_mutex; - std::map<MainLoop *, DAP *> dap_sessions; unsigned int clientCount = 0; - auto handle = listener->Accept(g_loop, [=, &dap_sessions_condition, - &dap_sessions_mutex, &dap_sessions, - &clientCount]( + auto handle = listener->Accept(g_loop, [=, &clientCount]( std::unique_ptr<Socket> sock) { // Reset the keep alive timer, because we won't be killing the server // while this connection is being served. @@ -339,8 +335,7 @@ static llvm::Error serveConnection( // Move the client into a background thread to unblock accepting the next // client. - std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex, - &dap_sessions]() { + std::thread client([=]() { llvm::set_thread_name(client_name + ".runloop"); MainLoop loop; Transport transport(client_name, log, io, io); @@ -353,10 +348,8 @@ static llvm::Error serveConnection( return; } - { - std::scoped_lock<std::mutex> lock(dap_sessions_mutex); - dap_sessions[&loop] = &dap; - } + // Register the DAP session with the global manager. + DAPSessionManager::GetInstance().RegisterSession(&loop, &dap); if (auto Err = dap.Loop()) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), @@ -365,10 +358,8 @@ static llvm::Error serveConnection( } DAP_LOG(log, "({0}) client disconnected", client_name); - std::unique_lock<std::mutex> lock(dap_sessions_mutex); - dap_sessions.erase(&loop); - std::notify_all_at_thread_exit(dap_sessions_condition, std::move(lock)); - + // Unregister the DAP session from the global manager. + DAPSessionManager::GetInstance().UnregisterSession(&loop); // Start the countdown to kill the server at the end of each connection. if (connection_timeout_seconds) TrackConnectionTimeout(g_loop, g_connection_timeout_mutex, @@ -391,29 +382,11 @@ static llvm::Error serveConnection( log, "lldb-dap server shutdown requested, disconnecting remaining clients..."); - bool client_failed = false; - { - std::scoped_lock<std::mutex> lock(dap_sessions_mutex); - for (auto [loop, dap] : dap_sessions) { - if (llvm::Error error = dap->Disconnect()) { - client_failed = true; - llvm::WithColor::error() << "DAP client disconnected failed: " - << llvm::toString(std::move(error)) << "\n"; - } - loop->AddPendingCallback( - [](MainLoopBase &loop) { loop.RequestTermination(); }); - } - } - - // Wait for all clients to finish disconnecting. - std::unique_lock<std::mutex> lock(dap_sessions_mutex); - dap_sessions_condition.wait(lock, [&] { return dap_sessions.empty(); }); + // Disconnect all active sessions using the global manager. + DAPSessionManager::GetInstance().DisconnectAllSessions(); - if (client_failed) - return llvm::make_error<llvm::StringError>( - "disconnecting all clients failed", llvm::inconvertibleErrorCode()); - - return llvm::Error::success(); + // Wait for all clients to finish disconnecting and return any errors. + return DAPSessionManager::GetInstance().WaitForAllSessionsToDisconnect(); } int main(int argc, char *argv[]) { @@ -641,6 +614,10 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } + // Register the DAP session with the global manager for stdio mode. + // This is needed for the event handling to find the correct DAP instance. + DAPSessionManager::GetInstance().RegisterSession(&loop, &dap); + // used only by TestVSCode_redirection_to_console.py if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr) redirection_test(); @@ -650,7 +627,9 @@ int main(int argc, char *argv[]) { llvm::toStringWithoutConsuming(Err)); llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "DAP session error: "); + DAPSessionManager::GetInstance().UnregisterSession(&loop); return EXIT_FAILURE; } + DAPSessionManager::GetInstance().UnregisterSession(&loop); return EXIT_SUCCESS; } diff --git a/lldb/unittests/DAP/CMakeLists.txt b/lldb/unittests/DAP/CMakeLists.txt index a08414c30e6cd..cc81479c7f024 100644 --- a/lldb/unittests/DAP/CMakeLists.txt +++ b/lldb/unittests/DAP/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(DAPTests DAPErrorTest.cpp + DAPSessionManagerTest.cpp DAPTest.cpp DAPTypesTest.cpp FifoFilesTest.cpp diff --git a/lldb/unittests/DAP/DAPSessionManagerTest.cpp b/lldb/unittests/DAP/DAPSessionManagerTest.cpp new file mode 100644 index 0000000000000..4b30d749d028b --- /dev/null +++ b/lldb/unittests/DAP/DAPSessionManagerTest.cpp @@ -0,0 +1,177 @@ +//===-- DAPSessionManagerTest.cpp ----------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DAPSessionManager.h" +#include "TestBase.h" +#include "lldb/API/SBDebugger.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace lldb_dap; +using namespace lldb; +using namespace lldb_dap_tests; + +class DAPSessionManagerTest : public DAPTestBase {}; + +TEST_F(DAPSessionManagerTest, GetInstanceReturnsSameSingleton) { + DAPSessionManager &instance1 = DAPSessionManager::GetInstance(); + DAPSessionManager &instance2 = DAPSessionManager::GetInstance(); + + EXPECT_EQ(&instance1, &instance2); +} + +TEST_F(DAPSessionManagerTest, SharedDebuggerSetAndGet) { + DAPSessionManager &manager = DAPSessionManager::GetInstance(); + + SBDebugger debugger = SBDebugger::Create(); + ASSERT_TRUE(debugger.IsValid()); + + uint32_t target_id = 12345; + manager.SetSharedDebugger(target_id, debugger); + + // Retrieval consumes the debugger (removes from map). + std::optional<SBDebugger> retrieved = manager.GetSharedDebugger(target_id); + ASSERT_TRUE(retrieved.has_value()); + EXPECT_TRUE(retrieved->IsValid()); + EXPECT_EQ(retrieved->GetID(), debugger.GetID()); + + // Second retrieval should fail. + std::optional<SBDebugger> second_retrieval = + manager.GetSharedDebugger(target_id); + EXPECT_FALSE(second_retrieval.has_value()); + + SBDebugger::Destroy(debugger); +} + +TEST_F(DAPSessionManagerTest, GetSharedDebuggerWithInvalidId) { + DAPSessionManager &manager = DAPSessionManager::GetInstance(); + + std::optional<SBDebugger> result = manager.GetSharedDebugger(99999); + + EXPECT_FALSE(result.has_value()); +} + +TEST_F(DAPSessionManagerTest, MultipleSharedDebuggersWithDifferentIds) { + DAPSessionManager &manager = DAPSessionManager::GetInstance(); + + SBDebugger debugger1 = SBDebugger::Create(); + SBDebugger debugger2 = SBDebugger::Create(); + ASSERT_TRUE(debugger1.IsValid()); + ASSERT_TRUE(debugger2.IsValid()); + + uint32_t target_id_1 = 1001; + uint32_t target_id_2 = 1002; + + manager.SetSharedDebugger(target_id_1, debugger1); + manager.SetSharedDebugger(target_id_2, debugger2); + + std::optional<SBDebugger> retrieved1 = manager.GetSharedDebugger(target_id_1); + ASSERT_TRUE(retrieved1.has_value()); + EXPECT_EQ(retrieved1->GetID(), debugger1.GetID()); + + std::optional<SBDebugger> retrieved2 = manager.GetSharedDebugger(target_id_2); + ASSERT_TRUE(retrieved2.has_value()); + EXPECT_EQ(retrieved2->GetID(), debugger2.GetID()); + + SBDebugger::Destroy(debugger1); + SBDebugger::Destroy(debugger2); +} + +TEST_F(DAPSessionManagerTest, CleanupSharedResources) { + DAPSessionManager &manager = DAPSessionManager::GetInstance(); + + SBDebugger debugger = SBDebugger::Create(); + ASSERT_TRUE(debugger.IsValid()); + manager.SetSharedDebugger(5555, debugger); + std::optional<SBDebugger> initial_result = manager.GetSharedDebugger(5555); + ASSERT_TRUE(initial_result.has_value()); + + manager.CleanupSharedResources(); + + std::optional<SBDebugger> result = manager.GetSharedDebugger(5555); + EXPECT_FALSE(result.has_value()); + + SBDebugger::Destroy(debugger); +} + +// UnregisterSession uses std::notify_all_at_thread_exit, so it must be called +// from a separate thread to properly release the mutex on thread exit. +TEST_F(DAPSessionManagerTest, RegisterAndUnregisterSession) { + DAPSessionManager &manager = DAPSessionManager::GetInstance(); + + // Initially not registered. + std::vector<DAP *> sessions_before = manager.GetActiveSessions(); + EXPECT_EQ( + std::count(sessions_before.begin(), sessions_before.end(), dap.get()), 0); + + manager.RegisterSession(&loop, dap.get()); + + // Should be in active sessions after registration. + std::vector<DAP *> sessions_after = manager.GetActiveSessions(); + EXPECT_EQ(std::count(sessions_after.begin(), sessions_after.end(), dap.get()), + 1); + + // Unregister. + std::thread unregister_thread([&]() { manager.UnregisterSession(&loop); }); + + unregister_thread.join(); + + // There should no longer be active sessions. + std::vector<DAP *> sessions_final = manager.GetActiveSessions(); + EXPECT_EQ(std::count(sessions_final.begin(), sessions_final.end(), dap.get()), + 0); +} + +TEST_F(DAPSessionManagerTest, DisconnectAllSessions) { + DAPSessionManager &manager = DAPSessionManager::GetInstance(); + + manager.RegisterSession(&loop, dap.get()); + + std::vector<DAP *> sessions = manager.GetActiveSessions(); + EXPECT_EQ(std::count(sessions.begin(), sessions.end(), dap.get()), 1); + + manager.DisconnectAllSessions(); + + // DisconnectAllSessions shutdown but doesn't wait for + // sessions to complete or remove them from the active sessions map. + sessions = manager.GetActiveSessions(); + EXPECT_EQ(std::count(sessions.begin(), sessions.end(), dap.get()), 1); + + std::thread unregister_thread([&]() { manager.UnregisterSession(&loop); }); + unregister_thread.join(); +} + +TEST_F(DAPSessionManagerTest, WaitForAllSessionsToDisconnect) { + DAPSessionManager &manager = DAPSessionManager::GetInstance(); + + manager.RegisterSession(&loop, dap.get()); + + std::vector<DAP *> sessions = manager.GetActiveSessions(); + EXPECT_EQ(std::count(sessions.begin(), sessions.end(), dap.get()), 1); + + // Unregister after a delay to test blocking behavior. + std::thread unregister_thread([&]() { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + manager.UnregisterSession(&loop); + }); + + // WaitForAllSessionsToDisconnect should block until unregistered. + auto start = std::chrono::steady_clock::now(); + llvm::Error err = manager.WaitForAllSessionsToDisconnect(); + EXPECT_FALSE(err); + auto duration = std::chrono::steady_clock::now() - start; + + // Verify it waited at least 100ms. + EXPECT_GE(duration, std::chrono::milliseconds(100)); + + // Session should be unregistered now. + sessions = manager.GetActiveSessions(); + EXPECT_EQ(std::count(sessions.begin(), sessions.end(), dap.get()), 0); + + unregister_thread.join(); +} diff --git a/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn b/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn index a42f781271a21..74e1e9bfc4a53 100644 --- a/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn +++ b/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn @@ -25,6 +25,7 @@ static_library("lib") { "DAP.cpp", "DAPError.cpp", "DAPLog.cpp", + "DAPSessionManager.cpp", "EventHelper.cpp", "ExceptionBreakpoint.cpp", "FifoFiles.cpp", >From 7ed64422592e514aee5f57b240c3d0afbe49a2e0 Mon Sep 17 00:00:00 2001 From: qxy11 <[email protected]> Date: Fri, 17 Oct 2025 00:24:12 -0700 Subject: [PATCH 2/3] Add GetSessionName, remove GetSessionNameFromEvent --- lldb/include/lldb/API/SBTarget.h | 12 ++++++++-- lldb/include/lldb/Target/Target.h | 37 +++++++++++++++++++++++-------- lldb/source/API/SBTarget.cpp | 16 ++++++------- lldb/source/Target/Target.cpp | 35 ++++------------------------- lldb/tools/lldb-dap/DAP.cpp | 3 +-- 5 files changed, 51 insertions(+), 52 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 1d251763a826a..640e0d80a06b3 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -70,8 +70,6 @@ class LLDB_API SBTarget { static lldb::SBModule GetModuleAtIndexFromEvent(const uint32_t idx, const lldb::SBEvent &event); - static const char *GetSessionNameFromEvent(const SBEvent &event); - static const char *GetBroadcasterClassName(); lldb::SBProcess GetProcess(); @@ -368,6 +366,16 @@ class LLDB_API SBTarget { /// LLDB_INVALID_GLOBALLY_UNIQUE_TARGET_ID if the target is invalid. lldb::user_id_t GetGloballyUniqueID() const; + /// Get the target session name for this target. + /// + /// The target session name provides a meaningful name for IDEs or tools to + /// display to help the user identify the origin and purpose of the target. + /// + /// \return + /// The target session name for this target, or nullptr if the target is + /// invalid or has no target session name. + const char *GetTargetSessionName() const; + SBError SetLabel(const char *label); /// Architecture opcode byte size width accessor diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 86349897762f4..1ce9499a60a43 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -557,11 +557,6 @@ class Target : public std::enable_shared_from_this<Target>, TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list); - TargetEventData(const lldb::TargetSP &target_sp, std::string session_name); - - TargetEventData(const lldb::TargetSP &target_sp, - const ModuleList &module_list, std::string session_name); - ~TargetEventData() override; static llvm::StringRef GetFlavorString(); @@ -570,8 +565,6 @@ class Target : public std::enable_shared_from_this<Target>, return TargetEventData::GetFlavorString(); } - static llvm::StringRef GetSessionNameFromEvent(const Event *event_ptr); - void Dump(Stream *s) const override; static const TargetEventData *GetEventDataFromEvent(const Event *event_ptr); @@ -587,7 +580,6 @@ class Target : public std::enable_shared_from_this<Target>, private: lldb::TargetSP m_target_sp; ModuleList m_module_list; - std::string m_session_name; TargetEventData(const TargetEventData &) = delete; const TargetEventData &operator=(const TargetEventData &) = delete; @@ -631,6 +623,30 @@ class Target : public std::enable_shared_from_this<Target>, /// requirements. llvm::Error SetLabel(llvm::StringRef label); + /// Get the target session name for this target. + /// + /// Provides a meaningful name for IDEs or tools to display for dynamically + /// created targets. Defaults to "Session {ID}" based on the globally unique + /// ID. + /// + /// \return + /// The target session name for this target. + const std::string &GetTargetSessionName() { return m_target_session_name; } + + /// Set the target session name for this target. + /// + /// This should typically be set along with the event + /// eBroadcastBitNewTargetCreated. Useful for scripts or triggers that + /// automatically create targets and want to provide meaningful names that + /// IDEs or other tools can display to help users identify the origin and + /// purpose of each target. + /// + /// \param[in] target_session_name + /// The target session name to set for this target. + void SetTargetSessionName(llvm::StringRef target_session_name) { + m_target_session_name = target_session_name.str(); + } + /// Find a binary on the system and return its Module, /// or return an existing Module that is already in the Target. /// @@ -1672,8 +1688,11 @@ class Target : public std::enable_shared_from_this<Target>, bool m_is_dummy_target; unsigned m_next_persistent_variable_index = 0; lldb::user_id_t m_target_unique_id = - LLDB_INVALID_GLOBALLY_UNIQUE_TARGET_ID; /// The globally unique ID + LLDB_INVALID_GLOBALLY_UNIQUE_TARGET_ID; ///< The globally unique ID /// assigned to this target + std::string m_target_session_name; ///< The target session name for this + /// target, used to name debugging + /// sessions in DAP. /// An optional \a lldb_private::Trace object containing processor trace /// information of this target. lldb::TraceSP m_trace_sp; diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 0f4508c772ee4..a4df4dea1233b 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -145,14 +145,6 @@ SBModule SBTarget::GetModuleAtIndexFromEvent(const uint32_t idx, return SBModule(module_list.GetModuleAtIndex(idx)); } -const char *SBTarget::GetSessionNameFromEvent(const SBEvent &event) { - LLDB_INSTRUMENT_VA(event); - - return ConstString( - Target::TargetEventData::GetSessionNameFromEvent(event.get())) - .AsCString(); -} - const char *SBTarget::GetBroadcasterClassName() { LLDB_INSTRUMENT(); @@ -1649,6 +1641,14 @@ lldb::user_id_t SBTarget::GetGloballyUniqueID() const { return LLDB_INVALID_GLOBALLY_UNIQUE_TARGET_ID; } +const char *SBTarget::GetTargetSessionName() const { + LLDB_INSTRUMENT_VA(this); + + if (TargetSP target_sp = GetSP()) + return ConstString(target_sp->GetTargetSessionName().data()).AsCString(); + return nullptr; +} + SBError SBTarget::SetLabel(const char *label) { LLDB_INSTRUMENT_VA(this, label); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index a13e11c4b3a05..46230dacc283e 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -186,6 +186,8 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, m_latest_stop_hook_id(0), m_valid(true), m_suppress_stop_hooks(false), m_is_dummy_target(is_dummy_target), m_target_unique_id(g_target_unique_id++), + m_target_session_name( + llvm::formatv("Session {0}", m_target_unique_id).str()), m_frame_recognizer_manager_up( std::make_unique<StackFrameRecognizerManager>()) { SetEventName(eBroadcastBitBreakpointChanged, "breakpoint-changed"); @@ -5170,21 +5172,11 @@ void TargetProperties::SetDebugUtilityExpression(bool debug) { // Target::TargetEventData Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp) - : TargetEventData(target_sp, ModuleList(), "") {} + : EventData(), m_target_sp(target_sp), m_module_list() {} Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list) - : TargetEventData(target_sp, module_list, "") {} - -Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, - std::string session_name) - : TargetEventData(target_sp, ModuleList(), std::move(session_name)) {} - -Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, - const ModuleList &module_list, - std::string session_name) - : EventData(), m_target_sp(target_sp), m_module_list(module_list), - m_session_name(std::move(session_name)) {} + : EventData(), m_target_sp(target_sp), m_module_list(module_list) {} Target::TargetEventData::~TargetEventData() = default; @@ -5220,25 +5212,6 @@ TargetSP Target::TargetEventData::GetTargetFromEvent(const Event *event_ptr) { return target_sp; } -llvm::StringRef -Target::TargetEventData::GetSessionNameFromEvent(const Event *event_ptr) { - const TargetEventData *event_data = GetEventDataFromEvent(event_ptr); - if (!event_data) - return llvm::StringRef(); - - if (!event_data->m_session_name.empty()) - return event_data->m_session_name; - - // Generate default session name if not provided - if (event_data->m_target_sp) { - lldb::user_id_t target_id = event_data->m_target_sp->GetGloballyUniqueID(); - std::string default_name = llvm::formatv("Session {0}", target_id).str(); - return ConstString(default_name).GetStringRef(); - } - - return llvm::StringRef(); -} - ModuleList Target::TargetEventData::GetModuleListFromEvent(const Event *event_ptr) { ModuleList module_list; diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index dc681336637e0..f346c897559d7 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1581,8 +1581,7 @@ void DAP::EventThread() { // process name. attach_config.try_emplace("type", "lldb"); attach_config.try_emplace("targetId", target_id); - const char *session_name = - lldb::SBTarget::GetSessionNameFromEvent(event); + const char *session_name = target.GetTargetSessionName(); attach_config.try_emplace("name", session_name); // 2. Construct the main 'startDebugging' request arguments. >From dea75d7ca48e36de42251a16123892714b33f0ef Mon Sep 17 00:00:00 2001 From: qxy11 <[email protected]> Date: Fri, 17 Oct 2025 00:27:15 -0700 Subject: [PATCH 3/3] Use StoreTargetById/RetrieveTargetById and add more comments --- .../tools/lldb-dap/attach/TestDAP_attach.py | 2 +- lldb/tools/lldb-dap/DAP.cpp | 18 ++-- lldb/tools/lldb-dap/DAPSessionManager.cpp | 33 +++---- lldb/tools/lldb-dap/DAPSessionManager.h | 34 +++++--- lldb/unittests/DAP/DAPSessionManagerTest.cpp | 87 +++++++++++++------ 5 files changed, 109 insertions(+), 65 deletions(-) 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 6ca286c12abd2..a49d233c6bd6e 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -83,5 +83,5 @@ def test_attach_with_invalid_targetId(self): resp = self.attach(targetId=99999, expectFailure=True) self.assertFalse(resp["success"]) self.assertIn( - "Unable to find existing debugger", resp["body"]["error"]["format"] + "Unable to find existing target", resp["body"]["error"]["format"] ) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index f346c897559d7..c5b96f5f5d611 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1312,14 +1312,15 @@ void DAP::StartEventThreads() { llvm::Error DAP::InitializeDebugger(std::optional<uint32_t> target_id) { // Initialize debugger instance (shared or individual). if (target_id) { - std::optional<lldb::SBDebugger> shared_debugger = - DAPSessionManager::GetInstance().GetSharedDebugger(*target_id); - // If the target ID is not valid, then we won't find a debugger. - if (!shared_debugger) { + std::optional<lldb::SBTarget> shared_target = + DAPSessionManager::GetInstance().TakeTargetById(*target_id); + // If the target ID is not valid, then we won't find a target. + if (!shared_target) { return llvm::createStringError( - "Unable to find existing debugger for target ID"); + "Unable to find existing target for target ID"); } - debugger = shared_debugger.value(); + // Get the debugger from the target and set it up. + debugger = shared_target->GetDebugger(); StartEventThreads(); return llvm::Error::success(); } @@ -1567,10 +1568,9 @@ void DAP::EventThread() { } else if (event_mask & lldb::SBTarget::eBroadcastBitNewTargetCreated) { auto target = lldb::SBTarget::GetTargetFromEvent(event); - // Generate unique target ID and set the shared debugger. + // Generate unique target ID and store the target for handoff. uint32_t target_id = target.GetGloballyUniqueID(); - DAPSessionManager::GetInstance().SetSharedDebugger(target_id, - debugger); + DAPSessionManager::GetInstance().StoreTargetById(target_id, target); // We create an attach config that will select the unique // target ID of the created target. The DAP instance will attach to diff --git a/lldb/tools/lldb-dap/DAPSessionManager.cpp b/lldb/tools/lldb-dap/DAPSessionManager.cpp index 5fd6e7e810fcc..b247ac0280b55 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.cpp +++ b/lldb/tools/lldb-dap/DAPSessionManager.cpp @@ -31,10 +31,10 @@ ManagedEventThread::~ManagedEventThread() { } DAPSessionManager &DAPSessionManager::GetInstance() { - // NOTE: intentional leak to avoid issues with C++ destructor chain - // Use std::call_once for thread-safe initialization. static std::once_flag initialized; - static DAPSessionManager *instance = nullptr; + static DAPSessionManager *instance = + nullptr; // NOTE: intentional leak to avoid issues with C++ destructor + // chain std::call_once(initialized, []() { instance = new DAPSessionManager(); }); @@ -104,7 +104,7 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, // Try to use shared event thread, if it exists. if (auto it = m_debugger_event_threads.find(debugger_id); it != m_debugger_event_threads.end()) { - if (auto thread_sp = it->second.lock()) + if (std::shared_ptr<ManagedEventThread> thread_sp = it->second.lock()) return thread_sp; // Our weak pointer has expired. m_debugger_event_threads.erase(it); @@ -118,21 +118,21 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, return new_thread_sp; } -void DAPSessionManager::SetSharedDebugger(uint32_t target_id, - lldb::SBDebugger debugger) { +void DAPSessionManager::StoreTargetById(uint32_t target_id, + lldb::SBTarget target) { std::lock_guard<std::mutex> lock(m_sessions_mutex); - m_target_to_debugger_map[target_id] = debugger; + m_target_map[target_id] = target; } -std::optional<lldb::SBDebugger> -DAPSessionManager::GetSharedDebugger(uint32_t target_id) { +std::optional<lldb::SBTarget> +DAPSessionManager::TakeTargetById(uint32_t target_id) { std::lock_guard<std::mutex> lock(m_sessions_mutex); - auto pos = m_target_to_debugger_map.find(target_id); - if (pos == m_target_to_debugger_map.end()) + auto pos = m_target_map.find(target_id); + if (pos == m_target_map.end()) return std::nullopt; - lldb::SBDebugger debugger = pos->second; - m_target_to_debugger_map.erase(pos); - return debugger; + lldb::SBTarget target = pos->second; + m_target_map.erase(pos); + return target; } DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { @@ -146,9 +146,10 @@ DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { } void DAPSessionManager::CleanupSharedResources() { - // SBDebugger destructors will handle cleanup when the map entries are + // Note: Caller must hold m_sessions_mutex. + // SBTarget destructors will handle cleanup when the map entries are // destroyed. - m_target_to_debugger_map.clear(); + m_target_map.clear(); } void DAPSessionManager::ReleaseExpiredEventThreads() { diff --git a/lldb/tools/lldb-dap/DAPSessionManager.h b/lldb/tools/lldb-dap/DAPSessionManager.h index d547f3036a9b2..d4115d56abc9f 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.h +++ b/lldb/tools/lldb-dap/DAPSessionManager.h @@ -43,7 +43,10 @@ class ManagedEventThread { std::thread m_event_thread; }; -/// Global DAP session manager. +/// Global DAP session manager that manages multiple concurrent DAP sessions in +/// a single lldb-dap process. Handles session lifecycle tracking, coordinates +/// shared debugger event threads, and facilitates target handoff between +/// sessions for dynamically created targets. class DAPSessionManager { public: /// Get the singleton instance of the DAP session manager. @@ -52,24 +55,27 @@ class DAPSessionManager { /// Register a DAP session. void RegisterSession(lldb_private::MainLoop *loop, DAP *dap); - /// Unregister a DAP session. + /// Unregister a DAP session. Called by sessions when they complete their + /// disconnection, which unblocks WaitForAllSessionsToDisconnect(). void UnregisterSession(lldb_private::MainLoop *loop); /// Get all active DAP sessions. std::vector<DAP *> GetActiveSessions(); - /// Disconnect all active sessions. + /// Disconnect all registered sessions by calling Disconnect() on + /// each and requesting their event loops to terminate. Used during + /// shutdown to force all sessions to begin disconnecting. void DisconnectAllSessions(); - /// Wait for all sessions to finish disconnecting. - /// Returns an error if any client disconnection failed, otherwise success. + /// Block until all sessions disconnect and unregister. Returns an error if + /// DisconnectAllSessions() was called and any disconnection failed. llvm::Error WaitForAllSessionsToDisconnect(); - /// Set the shared debugger instance for a unique target ID. - void SetSharedDebugger(uint32_t target_id, lldb::SBDebugger debugger); + /// Store a target for later retrieval by another session. + void StoreTargetById(uint32_t target_id, lldb::SBTarget target); - /// Get the shared debugger instance for a unique target ID. - std::optional<lldb::SBDebugger> GetSharedDebugger(uint32_t target_id); + /// Retrieve and remove a stored target by its globally unique target ID. + std::optional<lldb::SBTarget> TakeTargetById(uint32_t target_id); /// Get or create event thread for a specific debugger. std::shared_ptr<ManagedEventThread> @@ -104,12 +110,12 @@ class DAPSessionManager { std::condition_variable m_sessions_condition; std::map<lldb_private::MainLoop *, DAP *> m_active_sessions; - /// Optional map from target ID to shared debugger set when the native - /// process creates a new target. - std::map<uint32_t, lldb::SBDebugger> m_target_to_debugger_map; + /// Map from target ID to target for handing off newly created targets + /// between sessions. + std::map<uint32_t, lldb::SBTarget> m_target_map; - /// Map from debugger ID to its event thread used for when - /// multiple DAP sessions are using the same debugger instance. + /// Map from debugger ID to its event thread, used when multiple DAP sessions + /// share the same debugger instance. std::map<lldb::user_id_t, std::weak_ptr<ManagedEventThread>> m_debugger_event_threads; }; diff --git a/lldb/unittests/DAP/DAPSessionManagerTest.cpp b/lldb/unittests/DAP/DAPSessionManagerTest.cpp index 4b30d749d028b..52c52bf8c0569 100644 --- a/lldb/unittests/DAP/DAPSessionManagerTest.cpp +++ b/lldb/unittests/DAP/DAPSessionManagerTest.cpp @@ -25,58 +25,77 @@ TEST_F(DAPSessionManagerTest, GetInstanceReturnsSameSingleton) { EXPECT_EQ(&instance1, &instance2); } -TEST_F(DAPSessionManagerTest, SharedDebuggerSetAndGet) { +TEST_F(DAPSessionManagerTest, StoreAndTakeTarget) { DAPSessionManager &manager = DAPSessionManager::GetInstance(); SBDebugger debugger = SBDebugger::Create(); ASSERT_TRUE(debugger.IsValid()); + // Create an empty target (no executable) since we're only testing session + // management functionality, not actual debugging operations. + SBTarget target = debugger.CreateTarget(""); + ASSERT_TRUE(target.IsValid()); - uint32_t target_id = 12345; - manager.SetSharedDebugger(target_id, debugger); + uint32_t target_id = target.GetGloballyUniqueID(); + manager.StoreTargetById(target_id, target); - // Retrieval consumes the debugger (removes from map). - std::optional<SBDebugger> retrieved = manager.GetSharedDebugger(target_id); + // Retrieval consumes the target (removes from map). + std::optional<SBTarget> retrieved = manager.TakeTargetById(target_id); ASSERT_TRUE(retrieved.has_value()); EXPECT_TRUE(retrieved->IsValid()); - EXPECT_EQ(retrieved->GetID(), debugger.GetID()); + // Verify we got the correct target back. + EXPECT_EQ(retrieved->GetDebugger().GetID(), debugger.GetID()); + EXPECT_EQ(*retrieved, target); - // Second retrieval should fail. - std::optional<SBDebugger> second_retrieval = - manager.GetSharedDebugger(target_id); + // Second retrieval should fail (one-time retrieval semantics). + std::optional<SBTarget> second_retrieval = manager.TakeTargetById(target_id); EXPECT_FALSE(second_retrieval.has_value()); SBDebugger::Destroy(debugger); } -TEST_F(DAPSessionManagerTest, GetSharedDebuggerWithInvalidId) { +TEST_F(DAPSessionManagerTest, GetTargetWithInvalidId) { DAPSessionManager &manager = DAPSessionManager::GetInstance(); - std::optional<SBDebugger> result = manager.GetSharedDebugger(99999); + std::optional<SBTarget> result = manager.TakeTargetById(99999); EXPECT_FALSE(result.has_value()); } -TEST_F(DAPSessionManagerTest, MultipleSharedDebuggersWithDifferentIds) { +TEST_F(DAPSessionManagerTest, MultipleTargetsWithDifferentIds) { DAPSessionManager &manager = DAPSessionManager::GetInstance(); SBDebugger debugger1 = SBDebugger::Create(); SBDebugger debugger2 = SBDebugger::Create(); ASSERT_TRUE(debugger1.IsValid()); ASSERT_TRUE(debugger2.IsValid()); + // Create empty targets (no executable) since we're only testing session + // management functionality, not actual debugging operations. + SBTarget target1 = debugger1.CreateTarget(""); + SBTarget target2 = debugger2.CreateTarget(""); + ASSERT_TRUE(target1.IsValid()); + ASSERT_TRUE(target2.IsValid()); - uint32_t target_id_1 = 1001; - uint32_t target_id_2 = 1002; + uint32_t target_id_1 = target1.GetGloballyUniqueID(); + uint32_t target_id_2 = target2.GetGloballyUniqueID(); - manager.SetSharedDebugger(target_id_1, debugger1); - manager.SetSharedDebugger(target_id_2, debugger2); + // Sanity check the targets have distinct IDs. + EXPECT_NE(target_id_1, target_id_2); - std::optional<SBDebugger> retrieved1 = manager.GetSharedDebugger(target_id_1); + manager.StoreTargetById(target_id_1, target1); + manager.StoreTargetById(target_id_2, target2); + + std::optional<SBTarget> retrieved1 = manager.TakeTargetById(target_id_1); ASSERT_TRUE(retrieved1.has_value()); - EXPECT_EQ(retrieved1->GetID(), debugger1.GetID()); + EXPECT_TRUE(retrieved1->IsValid()); + // Verify we got the correct target by comparing debugger and target IDs. + EXPECT_EQ(retrieved1->GetDebugger().GetID(), debugger1.GetID()); + EXPECT_EQ(*retrieved1, target1); - std::optional<SBDebugger> retrieved2 = manager.GetSharedDebugger(target_id_2); + std::optional<SBTarget> retrieved2 = manager.TakeTargetById(target_id_2); ASSERT_TRUE(retrieved2.has_value()); - EXPECT_EQ(retrieved2->GetID(), debugger2.GetID()); + EXPECT_TRUE(retrieved2->IsValid()); + EXPECT_EQ(retrieved2->GetDebugger().GetID(), debugger2.GetID()); + EXPECT_EQ(*retrieved2, target2); SBDebugger::Destroy(debugger1); SBDebugger::Destroy(debugger2); @@ -87,14 +106,32 @@ TEST_F(DAPSessionManagerTest, CleanupSharedResources) { SBDebugger debugger = SBDebugger::Create(); ASSERT_TRUE(debugger.IsValid()); - manager.SetSharedDebugger(5555, debugger); - std::optional<SBDebugger> initial_result = manager.GetSharedDebugger(5555); - ASSERT_TRUE(initial_result.has_value()); + // Create empty targets (no executable) since we're only testing session + // management functionality, not actual debugging operations. + SBTarget target1 = debugger.CreateTarget(""); + SBTarget target2 = debugger.CreateTarget(""); + ASSERT_TRUE(target1.IsValid()); + ASSERT_TRUE(target2.IsValid()); + + uint32_t target_id_1 = target1.GetGloballyUniqueID(); + uint32_t target_id_2 = target2.GetGloballyUniqueID(); + + // Sanity check the targets have distinct IDs. + EXPECT_NE(target_id_1, target_id_2); + // Store multiple targets to verify cleanup removes them all. + manager.StoreTargetById(target_id_1, target1); + manager.StoreTargetById(target_id_2, target2); + + // Cleanup should remove all stored targets. manager.CleanupSharedResources(); - std::optional<SBDebugger> result = manager.GetSharedDebugger(5555); - EXPECT_FALSE(result.has_value()); + // Verify both targets are gone after cleanup. + std::optional<SBTarget> result1 = manager.TakeTargetById(target_id_1); + EXPECT_FALSE(result1.has_value()); + + std::optional<SBTarget> result2 = manager.TakeTargetById(target_id_2); + EXPECT_FALSE(result2.has_value()); SBDebugger::Destroy(debugger); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
