Author: Charles Zablit Date: 2025-12-23T09:28:11+01:00 New Revision: d8eec8e17a33693392448e314412c302e360daf3
URL: https://github.com/llvm/llvm-project/commit/d8eec8e17a33693392448e314412c302e360daf3 DIFF: https://github.com/llvm/llvm-project/commit/d8eec8e17a33693392448e314412c302e360daf3.diff LOG: [lldb-dap] refactor monitor thread in tests (#172879) This patch fixes a timeout in the monitor thread of the `test_by_name_waitFor` test. Currently, if `self.attach` fails, the `spawn_thread` will never finish and the test will eventually timeout after the 15mins timeout. We now ensure that we always join the thread at the end of the test. Additionally, this change also uses of the `spawnSubprocess` method to create the process. This should ensure the process is always properly cleaned up after an exception occurs. Added: Modified: lldb/packages/Python/lldbsuite/test/lldbtest.py lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py Removed: ################################################################################ diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index 8c1eea97620e2..67b94989f584e 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -42,6 +42,7 @@ import sys import time import traceback +from typing import Optional, Union # Third-party modules import unittest @@ -410,17 +411,22 @@ def __init__(self, trace_on): def pid(self): return self._proc.pid - def launch(self, executable, args, extra_env): + def launch(self, executable, args, extra_env, **kwargs): env = None if extra_env: env = dict(os.environ) env.update([kv.split("=", 1) for kv in extra_env]) + stdout = kwargs.pop("stdout", DEVNULL if not self._trace_on else None) + stderr = kwargs.pop("stderr", None) + self._proc = Popen( [executable] + args, - stdout=DEVNULL if not self._trace_on else None, + stdout=stdout, + stderr=stderr, stdin=PIPE, env=env, + **kwargs, ) def terminate(self): @@ -444,12 +450,20 @@ def terminate(self): self._proc.kill() time.sleep(self._delayafterterminate) + def communicate( + self, input: Optional[str] = None, timeout: Optional[float] = None + ) -> tuple[bytes, bytes]: + return self._proc.communicate(input, timeout) + def poll(self): return self._proc.poll() def wait(self, timeout=None): return self._proc.wait(timeout) + def kill(self): + return self._proc.kill() + class _RemoteProcess(_BaseProcess): def __init__(self, install_remote): @@ -460,7 +474,7 @@ def __init__(self, install_remote): def pid(self): return self._pid - def launch(self, executable, args, extra_env): + def launch(self, executable, args, extra_env, **kwargs): if self._install_remote: src_path = executable dst_path = lldbutil.join_remote_paths( @@ -943,16 +957,23 @@ def cleanupSubprocesses(self): del p del self.subprocesses[:] - def spawnSubprocess(self, executable, args=[], extra_env=None, install_remote=True): + @property + def lastSubprocess(self) -> Optional[Union[_RemoteProcess, _LocalProcess]]: + return self.subprocesses[-1] if len(self.subprocesses) > 0 else None + + def spawnSubprocess( + self, executable, args=None, extra_env=None, install_remote=True, **kwargs + ): """Creates a subprocess.Popen object with the specified executable and arguments, saves it in self.subprocesses, and returns the object. """ + args = [] if args is None else args proc = ( _RemoteProcess(install_remote) if lldb.remote_platform else _LocalProcess(self.TraceOn()) ) - proc.launch(executable, args, extra_env=extra_env) + proc.launch(executable, args, extra_env=extra_env, **kwargs) self.subprocesses.append(proc) return proc 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 c7174ce879606..00facfbc60ad5 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -15,10 +15,10 @@ # process scheduling can cause a massive (minutes) delay during this test. @skipIf(oslist=["linux"], archs=["arm$"]) class TestDAP_attach(lldbdap_testcase.DAPTestCaseBase): - def spawn(self, args): - self.process = subprocess.Popen( - args, - stdin=subprocess.PIPE, + def spawn(self, program, args=None): + return self.spawnSubprocess( + executable=program, + args=args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, @@ -26,21 +26,28 @@ def spawn(self, args): def spawn_and_wait(self, program, delay): time.sleep(delay) - self.spawn([program]) - self.process.wait() + proc = self.spawn(program=program) + # Wait for either the process to exit or the event to be set + while proc.poll() is None and not self.spawn_event.is_set(): + time.sleep(0.1) + proc.kill() + proc.wait() def continue_and_verify_pid(self): self.do_continue() - out, _ = self.process.communicate("foo") - self.assertIn(f"pid = {self.process.pid}", out) + proc = self.lastSubprocess + if proc is None: + self.fail(f"lastSubprocess is None") + out, _ = proc.communicate("foo") + self.assertIn(f"pid = {proc.pid}", out) def test_by_pid(self): """ Tests attaching to a process by process ID. """ program = self.build_and_create_debug_adapter_for_attach() - self.spawn([program]) - self.attach(pid=self.process.pid) + proc = self.spawn(program=program) + self.attach(pid=proc.pid) self.continue_and_verify_pid() def test_by_name(self): @@ -53,7 +60,7 @@ def test_by_name(self): pid_file_path = lldbutil.append_to_process_working_directory( self, "pid_file_%d" % (int(time.time())) ) - self.spawn([program, pid_file_path]) + self.spawn(program=program, args=[pid_file_path]) lldbutil.wait_for_file_on_target(self, pid_file_path) self.attach(program=program) @@ -66,6 +73,7 @@ def test_by_name_waitFor(self): doesn't exist yet. """ program = self.build_and_create_debug_adapter_for_attach() + self.spawn_event = threading.Event() self.spawn_thread = threading.Thread( target=self.spawn_and_wait, args=( @@ -74,8 +82,13 @@ def test_by_name_waitFor(self): ), ) self.spawn_thread.start() - self.attach(program=program, waitFor=True) - self.continue_and_verify_pid() + try: + self.attach(program=program, waitFor=True) + self.continue_and_verify_pid() + finally: + self.spawn_event.set() + if self.spawn_thread.is_alive(): + self.spawn_thread.join(timeout=10) def test_attach_with_missing_debuggerId_or_targetId(self): """ _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
