From 1f4181e2652f1de08aa4b1726251452e022a4b89 Mon Sep 17 00:00:00 2001 From: John Harrison Date: Mon, 3 Nov 2025 10:11:25 -0800 Subject: [PATCH] [lldb-dap] Addressing orphaned processes in tests. In lldb-dap tests, we sometimes spawn subprocesses directly but do not always correctly clean them up. This can cause some tests, like the TestDAP_disconnect.test_attach to hang and not properly respect timeouts. To fix this, I am passing the lldbtest.Base.spawnSubprocess helper to the adapter client so it can be used spawn subprocesses in a way that we can ensure they're cleaned up. --- .../test/tools/lldb-dap/dap_server.py | 75 ++++++++++--------- .../test/tools/lldb-dap/lldbdap_testcase.py | 1 + .../lldb-dap/disconnect/TestDAP_disconnect.py | 9 +-- .../tools/lldb-dap/server/TestDAP_server.py | 3 +- 4 files changed, 45 insertions(+), 43 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 d892c01f0bc71..ac550962cfb85 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 @@ -32,6 +32,10 @@ # timeout by a factor of 10 if ASAN is enabled. DEFAULT_TIMEOUT = 10 * (10 if ("ASAN_OPTIONS" in os.environ) else 1) +# See lldbtest.Base.spawnSubprocess, which should help ensure any processes +# created by the DAP client are terminated correctly when the test ends. +SpawnHelperCallback = Callable[[str, List[str], List[str]], subprocess.Popen] + ## DAP type references @@ -191,14 +195,16 @@ def __init__( self, recv: BinaryIO, send: BinaryIO, - init_commands: list[str], - log_file: Optional[TextIO] = None, + init_commands: Optional[List[str]] = None, + log_file: Optional[str] = None, + spawn_helper: Optional[SpawnHelperCallback] = None, ): # For debugging test failures, try setting `trace_file = sys.stderr`. self.trace_file: Optional[TextIO] = None self.log_file = log_file self.send = send self.recv = recv + self.spawn_helper = spawn_helper # Packets that have been received and processed but have not yet been # requested by a test case. @@ -211,7 +217,7 @@ def __init__( self._recv_thread = threading.Thread(target=self._read_packet_thread) # session state - self.init_commands = init_commands + self.init_commands = init_commands if init_commands else [] self.exit_status: Optional[int] = None self.capabilities: Dict = {} self.initialized: bool = False @@ -310,11 +316,6 @@ def collect_output( output += self.get_output(category, clear=clear) return output - def _enqueue_recv_packet(self, packet: Optional[ProtocolMessage]): - with self.recv_condition: - self.recv_packets.append(packet) - self.recv_condition.notify() - def _handle_recv_packet(self, packet: Optional[ProtocolMessage]) -> bool: """Handles an incoming packet. @@ -460,22 +461,11 @@ def _handle_reverse_request(self, request: Request) -> None: self.reverse_requests.append(request) arguments = request.get("arguments") if request["command"] == "runInTerminal" and arguments is not None: - in_shell = arguments.get("argsCanBeInterpretedByShell", False) - print("spawning...", arguments["args"]) - proc = subprocess.Popen( - arguments["args"], - env=arguments.get("env", {}), - cwd=arguments.get("cwd", None), - stdin=subprocess.DEVNULL, - stdout=sys.stderr, - stderr=sys.stderr, - shell=in_shell, - ) - body = {} - if in_shell: - body["shellProcessId"] = proc.pid - else: - body["processId"] = proc.pid + assert self.spawn_helper is not None, "Not configured to spawn subprocesses" + [exe, *args] = arguments["args"] + env = [f"{k}={v}" for k, v in arguments.get("env", {}).items()] + proc = self.spawn_helper(exe, args, env) + body = {"processId": proc.pid} self.send_packet( { "type": "response", @@ -1501,12 +1491,14 @@ def request_setInstructionBreakpoints(self, memory_reference=[]): class DebugAdapterServer(DebugCommunication): def __init__( self, + *, executable: Optional[str] = None, connection: Optional[str] = None, - init_commands: list[str] = [], - log_file: Optional[TextIO] = None, - env: Optional[dict[str, str]] = None, - additional_args: list[str] = [], + init_commands: Optional[list[str]] = None, + log_file: Optional[str] = None, + env: Optional[Dict[str, str]] = None, + additional_args: Optional[List[str]] = None, + spawn_helper: Optional[SpawnHelperCallback] = None, ): self.process = None self.connection = None @@ -1532,13 +1524,21 @@ def __init__( s = socket.create_connection((host.strip("[]"), int(port))) else: raise ValueError("invalid connection: {}".format(connection)) - DebugCommunication.__init__( - self, s.makefile("rb"), s.makefile("wb"), init_commands, log_file + super().__init__( + s.makefile("rb"), + s.makefile("wb"), + init_commands, + log_file, + spawn_helper, ) self.connection = connection else: - DebugCommunication.__init__( - self, self.process.stdout, self.process.stdin, init_commands, log_file + super().__init__( + self.process.stdout, + self.process.stdin, + init_commands, + log_file, + spawn_helper, ) @classmethod @@ -1546,14 +1546,14 @@ def launch( cls, *, executable: str, - env: Optional[dict[str, str]] = None, - log_file: Optional[TextIO] = None, + env: Optional[Dict[str, str]] = None, + log_file: Optional[str] = None, connection: Optional[str] = None, connection_timeout: Optional[int] = None, - additional_args: list[str] = [], + additional_args: Optional[List[str]] = None, ) -> tuple[subprocess.Popen, Optional[str]]: adapter_env = os.environ.copy() - if env is not None: + if env: adapter_env.update(env) if log_file: @@ -1561,7 +1561,8 @@ def launch( args = [executable] # Add additional arguments first (like --no-lldbinit) - args.extend(additional_args) + if additional_args: + args.extend(additional_args) if connection is not None: args.append("--connection") 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 c6c4a3e2a4e1e..71ca60ebe8d34 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 @@ -39,6 +39,7 @@ def create_debug_adapter( log_file=log_file_path, env=lldbDAPEnv, additional_args=additional_args or [], + spawn_helper=self.spawnSubprocess, ) def build_and_create_debug_adapter( diff --git a/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py b/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py index 09e3f62f0eead..19f88d88c2ff4 100644 --- a/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py +++ b/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py @@ -3,17 +3,15 @@ """ -import dap_server from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil import lldbdap_testcase -import subprocess import time import os -class TestDAP_launch(lldbdap_testcase.DAPTestCaseBase): +class TestDAP_disconnect(lldbdap_testcase.DAPTestCaseBase): source = "main.cpp" def disconnect_and_assert_no_output_printed(self): @@ -67,10 +65,11 @@ def test_attach(self): lambda: self.run_platform_command("rm %s" % (sync_file_path)) ) - self.process = subprocess.Popen([program, sync_file_path]) + proc = self.spawnSubprocess(program, [sync_file_path]) lldbutil.wait_for_file_on_target(self, sync_file_path) - self.attach(pid=self.process.pid, disconnectAutomatically=False) + self.attach(pid=proc.pid, disconnectAutomatically=False, stopOnEntry=True) + self.continue_to_next_stop() response = self.dap_server.request_evaluate("wait_for_attach = false;") self.assertTrue(response["success"]) diff --git a/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py b/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py index 12b321cf42778..3c53cf2ed3460 100644 --- a/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py +++ b/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py @@ -37,7 +37,7 @@ def cleanup(): def run_debug_session(self, connection, name, sleep_seconds_in_middle=None): self.dap_server = dap_server.DebugAdapterServer( - connection=connection, + connection=connection, spawn_helper=self.spawnSubprocess ) program = self.getBuildArtifact("a.out") source = "main.c" @@ -94,6 +94,7 @@ def test_server_interrupt(self): (process, connection) = self.start_server(connection="listen://localhost:0") self.dap_server = dap_server.DebugAdapterServer( connection=connection, + spawn_helper=self.spawnSubprocess, ) program = self.getBuildArtifact("a.out") source = "main.c"