https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/171549
>From 714a50cee306e1437ef38c08232700a164cdea55 Mon Sep 17 00:00:00 2001 From: John Harrison <[email protected]> Date: Tue, 9 Dec 2025 17:00:56 -0800 Subject: [PATCH 1/2] [lldb-dap] Adjusting the initialize/launch flow to better match the spec. In https://github.com/llvm/llvm-project/pull/170523 it was pointed out that the spec does specifically specify that launch/attach should not respond until configurationDone is handled. This means we do need to support async request handlers. To better align with the spec, I've added a new `lldb_dap::AsyncRequestHandler`. This is an additional handler type that allows us to respond at a later point. Additionally, I refactored `launch` and `attach` to only respond once the `configurationDone` is complete, specifically during the `PostRun` operation of the `configurationDone` handler. I merged some of the common behavior between `RequestHandler` and `AsyncRequestHandler` into their common `BaseRequestHandler`. --- .../test/tools/lldb-dap/dap_server.py | 27 ++-- .../test/tools/lldb-dap/lldbdap_testcase.py | 38 ++--- .../attach-commands/TestDAP_attachCommands.py | 2 +- .../tools/lldb-dap/attach/TestDAP_attach.py | 4 +- .../attach/TestDAP_attachByPortNum.py | 12 +- .../TestDAP_breakpointAssembly.py | 7 +- .../lldb-dap/commands/TestDAP_commands.py | 4 +- .../lldb-dap/coreFile/TestDAP_coreFile.py | 2 +- .../tools/lldb-dap/launch/TestDAP_launch.py | 26 ++-- .../runInTerminal/TestDAP_runInTerminal.py | 2 +- lldb/tools/lldb-dap/DAP.h | 2 + .../lldb-dap/Handler/AttachRequestHandler.cpp | 46 +++--- .../ConfigurationDoneRequestHandler.cpp | 10 +- .../lldb-dap/Handler/LaunchRequestHandler.cpp | 24 +-- .../tools/lldb-dap/Handler/RequestHandler.cpp | 46 +++++- lldb/tools/lldb-dap/Handler/RequestHandler.h | 138 ++++++++++-------- 16 files changed, 241 insertions(+), 149 deletions(-) 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 7a9d5a82983d7..dddcafc2630f8 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 @@ -46,7 +46,7 @@ class Event(TypedDict): body: Any -class Request(TypedDict, total=False): +class Request(TypedDict): type: Literal["request"] seq: int command: str @@ -477,6 +477,7 @@ def _handle_reverse_request(self, request: Request) -> None: "seq": 0, "request_seq": request["seq"], "success": True, + "message": None, "command": "runInTerminal", "body": body, } @@ -502,7 +503,7 @@ def _process_continued(self, all_threads_continued: bool): if all_threads_continued: self.thread_stop_reasons = {} - def _update_verified_breakpoints(self, breakpoints: list[Breakpoint]): + def _update_verified_breakpoints(self, breakpoints: List[Breakpoint]): for bp in breakpoints: # If no id is set, we cannot correlate the given breakpoint across # requests, ignore it. @@ -537,7 +538,7 @@ def send_packet(self, packet: ProtocolMessage) -> int: return packet["seq"] - def _send_recv(self, request: Request) -> Optional[Response]: + def _send_recv(self, request: Request) -> Response: """Send a command python dictionary as JSON and receive the JSON response. Validates that the response is the correct sequence and command in the reply. Any events that are received are added to the @@ -598,7 +599,7 @@ def wait_for_breakpoint_events(self): breakpoint_events.append(event) return breakpoint_events - def wait_for_breakpoints_to_be_verified(self, breakpoint_ids: list[str]): + def wait_for_breakpoints_to_be_verified(self, breakpoint_ids: List[str]): """Wait for all breakpoints to be verified. Return all unverified breakpoints.""" while any(id not in self.resolved_breakpoints for id in breakpoint_ids): breakpoint_event = self.wait_for_event(["breakpoint"]) @@ -800,7 +801,7 @@ def request_attach( sourceMap: Optional[Union[list[tuple[str, str]], dict[str, str]]] = None, gdbRemotePort: Optional[int] = None, gdbRemoteHostname: Optional[str] = None, - ): + ) -> int: args_dict = {} if pid is not None: args_dict["pid"] = pid @@ -838,7 +839,7 @@ def request_attach( if gdbRemoteHostname is not None: args_dict["gdb-remote-hostname"] = gdbRemoteHostname command_dict = {"command": "attach", "type": "request", "arguments": args_dict} - return self._send_recv(command_dict) + return self.send_packet(command_dict) def request_breakpointLocations( self, file_path, line, end_line=None, column=None, end_column=None @@ -872,7 +873,7 @@ def request_configurationDone(self): if response: self.configuration_done_sent = True stopped_on_entry = self.is_stopped - self.request_threads() + threads_response = self.request_threads() if not stopped_on_entry: # Drop the initial cached threads if we did not stop-on-entry. # In VSCode, immediately following 'configurationDone', a @@ -1000,10 +1001,10 @@ def request_evaluate( threadId=None, context=None, is_hex: Optional[bool] = None, - ): + ) -> Response: stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId) if stackFrame is None: - return [] + raise ValueError("invalid frameIndex") args_dict = { "expression": expression, "frameId": stackFrame["id"], @@ -1086,7 +1087,7 @@ def request_launch( commandEscapePrefix: Optional[str] = None, customFrameFormat: Optional[str] = None, customThreadFormat: Optional[str] = None, - ): + ) -> int: args_dict = {"program": program} if args: args_dict["args"] = args @@ -1137,7 +1138,7 @@ def request_launch( if commandEscapePrefix is not None: args_dict["commandEscapePrefix"] = commandEscapePrefix command_dict = {"command": "launch", "type": "request", "arguments": args_dict} - return self._send_recv(command_dict) + return self.send_packet(command_dict) def request_next(self, threadId, granularity="statement"): if self.exit_status is not None: @@ -1197,6 +1198,7 @@ def request_setBreakpoints(self, source: Source, line_array, data=None): Each parameter object is 1:1 mapping with entries in line_entry. It contains optional location/hitCondition/logMessage parameters. """ + assert self.initialized, "cannot setBreakpoints before initialized" args_dict = { "source": source, "sourceModified": False, @@ -1234,6 +1236,7 @@ def request_setBreakpoints(self, source: Source, line_array, data=None): def request_setExceptionBreakpoints( self, *, filters: list[str] = [], filter_options: list[dict] = [] ): + assert self.initialized, "cannot setExceptionBreakpoints before initialized" args_dict = {"filters": filters} if filter_options: args_dict["filterOptions"] = filter_options @@ -1245,6 +1248,7 @@ def request_setExceptionBreakpoints( return self._send_recv(command_dict) def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=None): + assert self.initialized, "cannot setFunctionBreakpoints before initialized" breakpoints = [] for name in names: bp = {"name": name} @@ -1293,6 +1297,7 @@ def request_setDataBreakpoint(self, dataBreakpoints): [hitCondition]: string } """ + assert self.initialized, "cannot setDataBreakpoints before initialized" args_dict = {"breakpoints": dataBreakpoints} command_dict = { "command": "setDataBreakpoints", 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 c7d302cc2dea2..53b2a03b30f84 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 @@ -4,7 +4,7 @@ import uuid import dap_server -from dap_server import Source +from dap_server import Source, Response from lldbsuite.test.decorators import skipIf from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbplatformutil @@ -497,9 +497,9 @@ def attach( *, disconnectAutomatically=True, sourceInitFile=False, - expectFailure=False, + waitForResponse=False, **kwargs, - ): + ) -> Optional[Response]: """Build the default Makefile target, create the DAP debug adapter, and attach to the process. """ @@ -515,22 +515,21 @@ def cleanup(): self.addTearDownHook(cleanup) # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) - response = self.dap_server.request_attach(**kwargs) - if expectFailure: - return response - if not (response and response["success"]): - error_msg = self._build_error_message("attach failed", response) - self.assertTrue(response and response["success"], error_msg) + attach_seq = self.dap_server.request_attach(**kwargs) + if waitForResponse: + return self.dap_server.receive_response(attach_seq) + self.dap_server.wait_for_event(["initialized"]) + return None def launch( self, - program=None, + program: str, *, sourceInitFile=False, disconnectAutomatically=True, - expectFailure=False, + waitForResponse=False, **kwargs, - ): + ) -> Optional[Response]: """Sending launch request to dap""" # Make sure we disconnect and terminate the DAP debug adapter, @@ -545,12 +544,11 @@ def cleanup(): # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) - response = self.dap_server.request_launch(program, **kwargs) - if expectFailure: - return response - if not (response and response["success"]): - error_msg = self._build_error_message("launch failed", response) - self.assertTrue(response and response["success"], error_msg) + launch_seq = self.dap_server.request_launch(program, **kwargs) + if waitForResponse: + return self.dap_server.receive_response(launch_seq) + self.dap_server.wait_for_event(["initialized"]) + return None def build_and_launch( self, @@ -567,6 +565,10 @@ def build_and_launch( return self.launch(program, **kwargs) + def verify_configuration_done(self): + resp = self.dap_server.request_configurationDone() + self.assertTrue(resp["success"]) + def getBuiltinDebugServerTool(self): # Tries to find simulation/lldb-server/gdbserver tool path. server_tool = None diff --git a/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py b/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py index 9e29f07db80f1..b4daf9fd171c2 100644 --- a/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py +++ b/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py @@ -105,7 +105,7 @@ def test_attach_command_process_failures(self): resp = self.attach( program=program, attachCommands=attachCommands, - expectFailure=True, + waitForResponse=True, ) self.assertFalse(resp["success"]) self.assertIn( 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 d6287397a93b0..c94de3a653dc9 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -84,7 +84,7 @@ def test_attach_with_missing_debuggerId_or_targetId(self): self.build_and_create_debug_adapter() # Test with only targetId specified (no debuggerId) - resp = self.attach(targetId=99999, expectFailure=True) + resp = self.attach(targetId=99999, waitForResponse=True) self.assertFalse(resp["success"]) self.assertIn( "Both debuggerId and targetId must be specified together", @@ -101,7 +101,7 @@ def test_attach_with_invalid_debuggerId_and_targetId(self): # Attach with both debuggerId=9999 and targetId=99999 (both invalid). # Since debugger ID 9999 likely doesn't exist in the global registry, # we expect a validation error. - resp = self.attach(debuggerId=9999, targetId=99999, expectFailure=True) + resp = self.attach(debuggerId=9999, targetId=99999, waitForResponse=True) self.assertFalse(resp["success"]) error_msg = resp["body"]["error"]["format"] # Either error is acceptable - both indicate the debugger reuse diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py index edb87a9314d78..e8f3f0fbc3fd3 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py @@ -99,7 +99,7 @@ def test_fails_if_both_port_and_pid_are_set(self): pid=pid, gdbRemotePort=port, sourceInitFile=True, - expectFailure=True, + waitForResponse=True, ) self.assertFalse( response["success"], "The user can't specify both pid and port" @@ -115,7 +115,10 @@ def test_by_invalid_port(self): port = 0 response = self.attach( - program=program, gdbRemotePort=port, sourceInitFile=True, expectFailure=True + program=program, + gdbRemotePort=port, + sourceInitFile=True, + waitForResponse=True, ) self.assertFalse( response["success"], @@ -138,7 +141,10 @@ def test_by_illegal_port(self): ) response = self.attach( - program=program, gdbRemotePort=port, sourceInitFile=True, expectFailure=True + program=program, + gdbRemotePort=port, + sourceInitFile=True, + waitForResponse=True, ) self.assertFalse( response["success"], diff --git a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py index fab109c93a17b..cbbea9ea9540b 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py @@ -94,9 +94,10 @@ def test_persistent_assembly_breakpoint(self): try: self.dap_server.request_initialize() self.dap_server.request_launch(program) + self.dap_server.wait_for_event(["initialized"]) - assmebly_func_breakpoints = self.set_function_breakpoints(["assembly_func"]) - self.continue_to_breakpoints(assmebly_func_breakpoints) + assembly_func_breakpoints = self.set_function_breakpoints(["assembly_func"]) + self.continue_to_breakpoints(assembly_func_breakpoints) assembly_func_frame = self.get_stackFrames()[0] source_reference = assembly_func_frame["source"]["sourceReference"] @@ -137,6 +138,8 @@ def test_persistent_assembly_breakpoint(self): try: self.dap_server.request_initialize() self.dap_server.request_launch(program) + self.dap_server.wait_for_event(["initialized"]) + new_session_breakpoints_ids = self.set_source_breakpoints_from_source( Source(persistent_breakpoint_source), [persistent_breakpoint_line], diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py index f53813a8a48f6..bde53f53ac318 100644 --- a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py +++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py @@ -47,7 +47,7 @@ def do_test_abort_on_error( launchCommands=commands if use_launch_commands else None, preRunCommands=commands if use_pre_run_commands else None, postRunCommands=commands if use_post_run_commands else None, - expectFailure=True, + waitForResponse=True, ) full_output = self.collect_console( pattern=command_abort_on_error, @@ -76,7 +76,7 @@ def test_command_directive_abort_on_error_attach_commands(self): resp = self.attach( program=program, attachCommands=["?!" + command_quiet, "!" + command_abort_on_error], - expectFailure=True, + waitForResponse=True, ) self.assertFalse(resp["success"], "expected 'attach' failure") full_output = self.collect_console(pattern=command_abort_on_error) diff --git a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py index d56a8a45ebf1e..174b61cbede57 100644 --- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py +++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py @@ -67,7 +67,7 @@ def test_wrong_core_file(self): self.create_debug_adapter() resp = self.attach( - program=exe_file, coreFile=wrong_core_file, expectFailure=True + program=exe_file, coreFile=wrong_core_file, waitForResponse=True ) self.assertIsNotNone(resp) self.assertFalse(resp["success"], "Expected failure in response {resp!r}") 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 ca881f1d817c5..b09803efc77c7 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -37,7 +37,7 @@ def test_failing_launch_program(self): """ program = self.getBuildArtifact("a.out") self.create_debug_adapter() - response = self.launch(program, expectFailure=True) + response = self.launch(program, waitForResponse=True) self.assertFalse(response["success"]) self.assertEqual( "'{0}' does not exist".format(program), response["body"]["error"]["format"] @@ -53,7 +53,7 @@ def test_failing_launch_commands_and_console(self): program, launchCommands=["a b c"], console="integratedTerminal", - expectFailure=True, + waitForResponse=True, ) self.assertFalse(response["success"]) self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"])) @@ -68,7 +68,7 @@ def test_failing_console(self): """ program = self.getBuildArtifact("a.out") self.create_debug_adapter() - response = self.launch(program, console="invalid", expectFailure=True) + response = self.launch(program, console="invalid", waitForResponse=True) self.assertFalse(response["success"]) self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"])) self.assertRegex( @@ -156,6 +156,7 @@ def test_debuggerRoot(self): self.build_and_launch( program, debuggerRoot=program_parent_dir, initCommands=commands ) + self.continue_to_exit() output = self.get_console() self.assertTrue(output and len(output) > 0, "expect console output") lines = output.splitlines() @@ -171,7 +172,6 @@ def test_debuggerRoot(self): % (program_parent_dir, line[len(prefix) :]), ) self.assertTrue(found, "verified lldb-dap working directory") - self.continue_to_exit() def test_sourcePath(self): """ @@ -180,6 +180,7 @@ def test_sourcePath(self): program = self.getBuildArtifact("a.out") program_dir = os.path.dirname(program) self.build_and_launch(program, sourcePath=program_dir) + self.continue_to_exit() output = self.get_console() self.assertTrue(output and len(output) > 0, "expect console output") lines = output.splitlines() @@ -195,7 +196,6 @@ def test_sourcePath(self): "lldb-dap working dir %s == %s" % (quoted_path, line[6:]), ) self.assertTrue(found, 'found "sourcePath" in console output') - self.continue_to_exit() @skipIfWindows def test_disableSTDIO(self): @@ -355,7 +355,7 @@ def test_commands(self): launch. "initCommands" are a list of LLDB commands that get executed - before the targt is created. + before the target is created. "preRunCommands" are a list of LLDB commands that get executed after the target has been created and before the launch. "stopCommands" are a list of LLDB commands that get executed each @@ -440,28 +440,28 @@ def test_extra_launch_commands(self): first_line = line_number(source, "// breakpoint 1") second_line = line_number(source, "// breakpoint 2") # Set target binary and 2 breakpoints - # then we can varify the "launchCommands" get run + # then we can verify the "launchCommands" get run # also we can verify that "stopCommands" get run as the # breakpoints get hit launchCommands = [ 'target create "%s"' % (program), - "breakpoint s -f main.c -l %d" % first_line, - "breakpoint s -f main.c -l %d" % second_line, "process launch --stop-at-entry", ] - initCommands = ["target list", "platform list"] preRunCommands = ["image list a.out", "image dump sections a.out"] + postRunCommands = ['script print("hello world")'] stopCommands = ["frame variable", "bt"] exitCommands = ["expr 2+3", "expr 3+4"] self.launch( program, initCommands=initCommands, preRunCommands=preRunCommands, + postRunCommands=postRunCommands, stopCommands=stopCommands, exitCommands=exitCommands, launchCommands=launchCommands, ) + self.set_source_breakpoints("main.c", [first_line, second_line]) # Get output from the console. This should contain both the # "initCommands" and the "preRunCommands". @@ -474,6 +474,7 @@ def test_extra_launch_commands(self): # Verify all "launchCommands" were found in console output # After execution, program should launch self.verify_commands("launchCommands", output, launchCommands) + self.verify_commands("postRunCommands", output, postRunCommands) # Verify the "stopCommands" here self.continue_to_next_stop() output = self.get_console() @@ -511,7 +512,7 @@ def test_failing_launch_commands(self): initCommands=initCommands, preRunCommands=preRunCommands, launchCommands=launchCommands, - expectFailure=True, + waitForResponse=True, ) self.assertFalse(response["success"]) @@ -608,7 +609,8 @@ def test_no_lldbinit_flag(self): initCommands = ["settings show stop-disassembly-display"] # Launch with initCommands to check the setting - self.launch(program, initCommands=initCommands, stopOnEntry=True) + self.launch(program, initCommands=initCommands) + self.continue_to_exit() # Get console output to verify the setting was NOT set from .lldbinit output = self.get_console() diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py index 4a360a0bcad01..146f80fd31a1f 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -108,7 +108,7 @@ def test_runInTerminalInvalidTarget(self): console="integratedTerminal", args=["foobar"], env=["FOO=bar"], - expectFailure=True, + waitForResponse=True, ) self.assertFalse(response["success"]) self.assertIn( diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 01139221ea37f..4db57c05362df 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -36,6 +36,7 @@ #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" @@ -145,6 +146,7 @@ struct DAP final : public DAPTransport::MessageHandler { ReplMode repl_mode; lldb::SBFormat frame_format; lldb::SBFormat thread_format; + llvm::unique_function<void()> on_configuration_done; /// This is used to allow request_evaluate to handle empty expressions /// (ie the user pressed 'return' and expects the previous expression to diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index f0996eb3ff0f4..822429564210a 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -15,9 +15,9 @@ #include "lldb/API/SBAttachInfo.h" #include "lldb/API/SBListener.h" #include "lldb/lldb-defines.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" -#include <cstdint> using namespace llvm; using namespace lldb_dap::protocol; @@ -29,38 +29,38 @@ 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 { +void AttachRequestHandler::Run(const AttachRequestArguments &args, + unique_function<void(Error)> callback) const { // Initialize DAP debugger and related components if not sharing previously // launched debugger. std::optional<int> debugger_id = args.debuggerId; std::optional<lldb::user_id_t> target_id = args.targetId; // Validate that both debugger_id and target_id are provided together. - if (debugger_id.has_value() != target_id.has_value()) { - return llvm::createStringError( + if (debugger_id.has_value() != target_id.has_value()) + return callback(llvm::createStringError( "Both debuggerId and targetId must be specified together for debugger " - "reuse, or both must be omitted to create a new debugger"); - } + "reuse, or both must be omitted to create a new debugger")); if (Error err = debugger_id && target_id ? dap.InitializeDebugger(*debugger_id, *target_id) : dap.InitializeDebugger()) - return err; + return callback(std::move(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 && !target_id.has_value()) - return make_error<DAPError>( + return callback(make_error<DAPError>( "expected one of 'pid', 'program', 'attachCommands', " - "'coreFile', 'gdb-remote-port', or target_id 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) && (args.gdbRemotePort != LLDB_DAP_INVALID_PORT)) - return make_error<DAPError>( - "'pid' and 'gdb-remote-port' are mutually exclusive"); + return callback(make_error<DAPError>( + "'pid' and 'gdb-remote-port' are mutually exclusive")); dap.SetConfiguration(args.configuration, /*is_attach=*/true); if (!args.coreFile.empty()) @@ -77,7 +77,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { // Run any initialize LLDB commands the user specified in the launch.json if (llvm::Error err = dap.RunInitCommands()) - return err; + return callback(std::move(err)); dap.ConfigureSourceMaps(); @@ -97,13 +97,13 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { } if (error.Fail()) - return ToError(error); + return callback(ToError(error)); dap.SetTarget(target); // Run any pre run LLDB commands the user specified in the launch.json if (Error err = dap.RunPreRunCommands()) - return err; + return callback(std::move(err)); if ((args.pid == LLDB_INVALID_PROCESS_ID || args.gdbRemotePort == LLDB_DAP_INVALID_PORT) && @@ -125,14 +125,14 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { // user that their command failed or the debugger is in an unexpected // state. if (llvm::Error err = dap.RunAttachCommands(args.attachCommands)) - return err; + return callback(std::move(err)); dap.target = dap.debugger.GetSelectedTarget(); // Validate the attachCommand results. if (!dap.target.GetProcess().IsValid()) - return make_error<DAPError>( - "attachCommands failed to attach to a process"); + return callback(make_error<DAPError>( + "attachCommands failed to attach to a process")); } else if (!args.coreFile.empty()) { dap.target.LoadCore(args.coreFile.data(), error); } else if (args.gdbRemotePort != LLDB_DAP_INVALID_PORT) { @@ -156,23 +156,23 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { dap.target.Attach(attach_info, error); } if (error.Fail()) - return ToError(error); + return callback(ToError(error)); } // Make sure the process is attached and stopped. error = dap.WaitForProcessToStop(args.configuration.timeout); if (error.Fail()) - return ToError(error); + return callback(ToError(error)); if (args.coreFile.empty() && !dap.target.GetProcess().IsValid()) - return make_error<DAPError>("failed to attach to process"); + return callback(make_error<DAPError>("failed to attach to process")); dap.RunPostRunCommands(); - return Error::success(); -} + dap.on_configuration_done = [callback = std::move(callback)]() mutable { + callback(Error::success()); + }; -void AttachRequestHandler::PostRun() const { dap.SendJSON(CreateEventObject("initialized")); } diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp index 1bfe7b7f6ef5c..ec089a1d91ddd 100644 --- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp @@ -9,7 +9,6 @@ #include "DAP.h" #include "EventHelper.h" #include "LLDBUtils.h" -#include "Protocol/ProtocolEvents.h" #include "Protocol/ProtocolRequests.h" #include "ProtocolUtils.h" #include "RequestHandler.h" @@ -66,4 +65,13 @@ ConfigurationDoneRequestHandler::Run(const ConfigurationDoneArguments &) const { return ToError(process.Continue()); } +void ConfigurationDoneRequestHandler::PostRun() const { + if (dap.on_configuration_done) { + dap.on_configuration_done(); + + // Clear the callback to ensure any captured resources are released. + dap.on_configuration_done = nullptr; + } +} + } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 329f0a7bf6453..f4214cef43b19 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -21,16 +21,18 @@ using namespace lldb_dap::protocol; namespace lldb_dap { /// Launch request; value of command field is 'launch'. -Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { +void LaunchRequestHandler::Run( + const LaunchRequestArguments &arguments, + llvm::unique_function<void(llvm::Error)> callback) const { // Initialize DAP debugger. if (Error err = dap.InitializeDebugger()) - return err; + return callback(std::move(err)); // Validate that we have a well formed launch request. if (!arguments.launchCommands.empty() && arguments.console != protocol::eConsoleInternal) - return make_error<DAPError>( - "'launchCommands' and non-internal 'console' are mutually exclusive"); + return callback(make_error<DAPError>( + "'launchCommands' and non-internal 'console' are mutually exclusive")); dap.SetConfiguration(arguments.configuration, /*is_attach=*/false); dap.last_launch_request = arguments; @@ -48,30 +50,30 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { // This is run before target is created, so commands can't do anything with // the targets - preRunCommands are run with the target. if (Error err = dap.RunInitCommands()) - return err; + return callback(std::move(err)); dap.ConfigureSourceMaps(); lldb::SBError error; lldb::SBTarget target = dap.CreateTarget(error); if (error.Fail()) - return ToError(error); + return callback(ToError(error)); dap.SetTarget(target); // Run any pre run LLDB commands the user specified in the launch.json if (Error err = dap.RunPreRunCommands()) - return err; + return callback(std::move(err)); if (Error err = LaunchProcess(arguments)) - return err; + return callback(std::move(err)); dap.RunPostRunCommands(); - return Error::success(); -} + dap.on_configuration_done = [callback = std::move(callback)]() mutable { + callback(Error::success()); + }; -void LaunchRequestHandler::PostRun() const { dap.SendJSON(CreateEventObject("initialized")); } diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index d67437ad5b3ae..4429817406c3d 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -257,7 +257,7 @@ llvm::Error BaseRequestHandler::LaunchProcess( void BaseRequestHandler::PrintWelcomeMessage() const { #ifdef LLDB_DAP_WELCOME_MESSAGE - dap.SendOutput(OutputType::Console, LLDB_DAP_WELCOME_MESSAGE); + dap.SendOutput(eOutputCategoryConsole, LLDB_DAP_WELCOME_MESSAGE); #endif } @@ -268,4 +268,48 @@ bool BaseRequestHandler::HasInstructionGranularity( return false; } +void BaseRequestHandler::HandleErrorResponse( + llvm::Error err, protocol::Response &response) const { + response.success = false; + llvm::handleAllErrors( + std::move(err), + [&](const NotStoppedError &err) { + response.message = lldb_dap::protocol::eResponseMessageNotStopped; + }, + [&](const DAPError &err) { + protocol::ErrorMessage error_message; + error_message.sendTelemetry = false; + error_message.format = err.getMessage(); + error_message.showUser = err.getShowUser(); + error_message.id = err.convertToErrorCode().value(); + error_message.url = err.getURL(); + error_message.urlLabel = err.getURLLabel(); + protocol::ErrorResponseBody body; + body.error = error_message; + response.body = body; + }, + [&](const llvm::ErrorInfoBase &err) { + protocol::ErrorMessage error_message; + error_message.showUser = true; + error_message.sendTelemetry = false; + error_message.format = err.message(); + error_message.id = err.convertToErrorCode().value(); + protocol::ErrorResponseBody body; + body.error = error_message; + response.body = body; + }); +} + +void BaseRequestHandler::Send(protocol::Response &response) const { + // Mark the request as 'cancelled' if the debugger was interrupted while + // evaluating this handler. + if (dap.debugger.InterruptRequested()) { + response.success = false; + response.message = protocol::eResponseMessageCancelled; + response.body = std::nullopt; + } + + dap.Send(response); +} + } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index fdce33de3f680..f482739266ee0 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -11,17 +11,16 @@ #include "DAP.h" #include "DAPError.h" -#include "DAPLog.h" #include "Protocol/ProtocolBase.h" #include "Protocol/ProtocolRequests.h" #include "Protocol/ProtocolTypes.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" #include <optional> #include <type_traits> -#include <variant> #include <vector> template <typename T> struct is_optional : std::false_type {}; @@ -76,6 +75,12 @@ class BaseRequestHandler { /// @} + /// HandleErrorResponse converts an error into a response. + void HandleErrorResponse(llvm::Error err, protocol::Response &response) const; + + /// Send a response to the client. + void Send(protocol::Response &response) const; + DAP &dap; }; @@ -135,36 +140,26 @@ class RequestHandler : public BaseRequestHandler { llvm::Expected<Args> arguments = parseArgs<Args>(request); if (llvm::Error err = arguments.takeError()) { HandleErrorResponse(std::move(err), response); - dap.Send(response); + Send(response); return; } if constexpr (std::is_same_v<Resp, llvm::Error>) { - if (llvm::Error err = Run(*arguments)) { + if (llvm::Error err = Run(*arguments)) HandleErrorResponse(std::move(err), response); - } else { + else response.success = true; - } } else { Resp body = Run(*arguments); - if (llvm::Error err = body.takeError()) { + if (llvm::Error err = body.takeError()) HandleErrorResponse(std::move(err), response); - } else { + else { response.success = true; response.body = std::move(*body); } } - // Mark the request as 'cancelled' if the debugger was interrupted while - // evaluating this handler. - if (dap.debugger.InterruptRequested()) { - dap.debugger.CancelInterruptRequest(); - response.success = false; - response.message = protocol::eResponseMessageCancelled; - response.body = std::nullopt; - } - - dap.Send(response); + Send(response); PostRun(); }; @@ -177,48 +172,71 @@ class RequestHandler : public BaseRequestHandler { /// *NOTE*: PostRun will be invoked even if the `Run` operation returned an /// error. virtual void PostRun() const {}; +}; - void HandleErrorResponse(llvm::Error err, - protocol::Response &response) const { - response.success = false; - llvm::handleAllErrors( - std::move(err), - [&](const NotStoppedError &err) { - response.message = lldb_dap::protocol::eResponseMessageNotStopped; - }, - [&](const DAPError &err) { - protocol::ErrorMessage error_message; - error_message.sendTelemetry = false; - error_message.format = err.getMessage(); - error_message.showUser = err.getShowUser(); - error_message.id = err.convertToErrorCode().value(); - error_message.url = err.getURL(); - error_message.urlLabel = err.getURLLabel(); - protocol::ErrorResponseBody body; - body.error = error_message; - response.body = body; - }, - [&](const llvm::ErrorInfoBase &err) { - protocol::ErrorMessage error_message; - error_message.showUser = true; - error_message.sendTelemetry = false; - error_message.format = err.message(); - error_message.id = err.convertToErrorCode().value(); - protocol::ErrorResponseBody body; - body.error = error_message; - response.body = body; - }); - } +/// Base class for handling DAP requests. Handlers should declare their +/// arguments and response body types like: +/// +/// class MyRequestHandler : public AsyncRequestHandler<Arguments, Response> { +/// .... +/// }; +template <typename Args, typename Resp> +class AsyncRequestHandler : public BaseRequestHandler { + using BaseRequestHandler::BaseRequestHandler; + + void operator()(const protocol::Request &request) const override { + protocol::Response response; + response.request_seq = request.seq; + response.command = request.command; + + llvm::Expected<Args> arguments = parseArgs<Args>(request); + if (llvm::Error err = arguments.takeError()) { + HandleErrorResponse(std::move(err), response); + dap.Send(response); + return; + } + + if constexpr (std::is_same_v<Resp, llvm::Error>) { + Run(*arguments, [this, response](llvm::Error err) mutable { + lldb::SBMutex lock = dap.GetAPIMutex(); + std::lock_guard<lldb::SBMutex> guard(lock); + + if (err) + HandleErrorResponse(std::move(err), response); + else + response.success = true; + + Send(response); + }); + } else { + Run(*arguments, [this, response](Resp body) mutable { + lldb::SBMutex lock = dap.GetAPIMutex(); + std::lock_guard<lldb::SBMutex> guard(lock); + + if (llvm::Error err = body.takeError()) + HandleErrorResponse(std::move(err), response); + else { + response.success = true; + response.body = std::move(*body); + } + + Send(response); + }); + } + }; + + virtual void Run(const Args &, llvm::unique_function<void(Resp)>) const = 0; }; class AttachRequestHandler - : public RequestHandler<protocol::AttachRequestArguments, - protocol::AttachResponse> { + : public AsyncRequestHandler<protocol::AttachRequestArguments, + protocol::AttachResponse> { public: - using RequestHandler::RequestHandler; + using AsyncRequestHandler::AsyncRequestHandler; static llvm::StringLiteral GetCommand() { return "attach"; } - llvm::Error Run(const protocol::AttachRequestArguments &args) const override; - void PostRun() const override; + void + Run(const protocol::AttachRequestArguments &args, + llvm::unique_function<void(protocol::AttachResponse)>) const override; }; class BreakpointLocationsRequestHandler @@ -277,6 +295,7 @@ class ConfigurationDoneRequestHandler } protocol::ConfigurationDoneResponse Run(const protocol::ConfigurationDoneArguments &) const override; + void PostRun() const override; }; class DisconnectRequestHandler @@ -330,14 +349,13 @@ class InitializeRequestHandler }; class LaunchRequestHandler - : public RequestHandler<protocol::LaunchRequestArguments, - protocol::LaunchResponse> { + : public AsyncRequestHandler<protocol::LaunchRequestArguments, + protocol::LaunchResponse> { public: - using RequestHandler::RequestHandler; + using AsyncRequestHandler::AsyncRequestHandler; static llvm::StringLiteral GetCommand() { return "launch"; } - llvm::Error - Run(const protocol::LaunchRequestArguments &arguments) const override; - void PostRun() const override; + void Run(const protocol::LaunchRequestArguments &arguments, + llvm::unique_function<void(llvm::Error)>) const override; }; class RestartRequestHandler : public LegacyRequestHandler { >From 56866b5a5e90c18a19b124817e9c8dfd086deff4 Mon Sep 17 00:00:00 2001 From: John Harrison <[email protected]> Date: Thu, 11 Dec 2025 09:12:25 -0800 Subject: [PATCH 2/2] Apply suggestion from @JDevlieghere Co-authored-by: Jonas Devlieghere <[email protected]> --- lldb/tools/lldb-dap/Handler/RequestHandler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index f482739266ee0..3730c53890636 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -213,9 +213,9 @@ class AsyncRequestHandler : public BaseRequestHandler { lldb::SBMutex lock = dap.GetAPIMutex(); std::lock_guard<lldb::SBMutex> guard(lock); - if (llvm::Error err = body.takeError()) + if (llvm::Error err = body.takeError()) { HandleErrorResponse(std::move(err), response); - else { + } else { response.success = true; response.body = std::move(*body); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
