From a85bc9bcd8f30f5940ecfb8fd865ae79c17de45a Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 01:52:43 -0400 Subject: [PATCH 1/9] Fix another place where _colorize was checking the server's tty instead of the client's --- Lib/_pyrepl/utils.py | 11 +++++++---- Lib/pdb.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Lib/_pyrepl/utils.py b/Lib/_pyrepl/utils.py index dd327d6990234c..7dc6f879a1e93a 100644 --- a/Lib/_pyrepl/utils.py +++ b/Lib/_pyrepl/utils.py @@ -23,9 +23,9 @@ BUILTINS = {str(name) for name in dir(builtins) if not name.startswith('_')} -def THEME(): +def THEME(**kwargs): # Not cached: the user can modify the theme inside the interactive session. - return _colorize.get_theme().syntax + return _colorize.get_theme(**kwargs).syntax class Span(NamedTuple): @@ -254,7 +254,10 @@ def is_soft_keyword_used(*tokens: TI | None) -> bool: def disp_str( - buffer: str, colors: list[ColorSpan] | None = None, start_index: int = 0 + buffer: str, + colors: list[ColorSpan] | None = None, + start_index: int = 0, + force_color=False, ) -> tuple[CharBuffer, CharWidths]: r"""Decompose the input buffer into a printable variant with applied colors. @@ -295,7 +298,7 @@ def disp_str( # move past irrelevant spans colors.pop(0) - theme = THEME() + theme = THEME(force_color=force_color) pre_color = "" post_color = "" if colors and colors[0].span.start < start_index: diff --git a/Lib/pdb.py b/Lib/pdb.py index 4efda171b7a813..f89d104fcddb9a 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -1069,7 +1069,7 @@ def handle_command_def(self, line): def _colorize_code(self, code): if self.colorize: colors = list(_pyrepl.utils.gen_colors(code)) - chars, _ = _pyrepl.utils.disp_str(code, colors=colors) + chars, _ = _pyrepl.utils.disp_str(code, colors=colors, force_color=True) code = "".join(chars) return code From 78f54f86021a11fd0a35758a0411ffd66c209244 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 01:52:53 -0400 Subject: [PATCH 2/9] Add an integration test --- Lib/test/test_remote_pdb.py | 132 +++++++++++++++++++++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 7c77bedb766c53..d882fbb455a21a 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -3,6 +3,7 @@ import itertools import json import os +import re import signal import socket import subprocess @@ -14,7 +15,7 @@ import unittest.mock from contextlib import closing, contextmanager, redirect_stdout, ExitStack from pathlib import Path -from test.support import is_wasi, os_helper, requires_subprocess, SHORT_TIMEOUT +from test.support import is_wasi, cpython_only, force_color, requires_subprocess, SHORT_TIMEOUT from test.support.os_helper import temp_dir, TESTFN, unlink from typing import Dict, List, Optional, Tuple, Union, Any @@ -1431,5 +1432,134 @@ def test_multi_line_commands(self): self.assertIn("Function returned: 42", stdout) self.assertEqual(process.returncode, 0) + +def _supports_remote_attaching(): + from contextlib import suppress + PROCESS_VM_READV_SUPPORTED = False + + try: + from _remote_debugging import PROCESS_VM_READV_SUPPORTED + except ImportError: + pass + + return PROCESS_VM_READV_SUPPORTED + + +@unittest.skipIf(not sys.is_remote_debug_enabled(), "Remote debugging is not enabled") +@unittest.skipIf(sys.platform != "darwin" and sys.platform != "linux" and sys.platform != "win32", + "Test only runs on Linux, Windows and MacOS") +@unittest.skipIf(sys.platform == "linux" and not _supports_remote_attaching(), + "Testing on Linux requires process_vm_readv support") +@cpython_only +@requires_subprocess() +class PdbAttachTestCase(unittest.TestCase): + def setUp(self): + # Create a server socket that will wait for the debugger to connect + self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self.sock.bind(('127.0.0.1', 0)) # Let OS assign port + self.sock.listen(1) + self.port = self.sock.getsockname()[1] + self._create_script() + + def _create_script(self, script=None): + # Create a file for subprocess script + script = textwrap.dedent( + f""" + import socket + import time + + def foo(): + return bar() + + def bar(): + return baz() + + def baz(): + x = 1 + # Trigger attach + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.connect(('127.0.0.1', {self.port})) + sock.close() + count = 0 + while x == 1 and count < 100: + count += 1 + time.sleep(0.1) + return x + + result = foo() + print(f"Function returned: {{result}}") + """ + ) + + self.script_path = TESTFN + "_connect_test.py" + with open(self.script_path, 'w') as f: + f.write(script) + + def tearDown(self): + self.sock.close() + try: + unlink(self.script_path) + except OSError: + pass + + def do_integration_test(self, client_stdin): + process = subprocess.Popen( + [sys.executable, self.script_path], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + # Wait for the process to reach our attachment point + self.sock.settimeout(10) + conn, _ = self.sock.accept() + conn.close() + + attach_process = subprocess.Popen( + [sys.executable, "-m", "pdb", "-p", str(process.pid)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + stdin=subprocess.PIPE, + text=True, + env=os.environ, + ) + stdout, stderr = attach_process.communicate(client_stdin, timeout=5) + process.wait() + + self.assertEqual(process.returncode, 0) + self.assertEqual(attach_process.returncode, 0) + + return { + "client": { + "stdout": stdout, + "stderr": stderr, + }, + "server": { + "stdout": process.stdout.read(), + "stderr": process.stderr.read(), + }, + } + + def test_attach_to_process_without_colors(self): + with force_color(False): + output = self.do_integration_test("ll\nx=42\n") + self.assertEqual(output["client"]["stderr"], "") + self.assertEqual(output["server"]["stderr"], "") + + self.assertEqual(output["server"]["stdout"], "Function returned: 42\n") + self.assertIn("while x == 1", output["client"]["stdout"]) + self.assertNotIn("\x1b", output["client"]["stdout"]) + + def test_attach_to_process_with_colors(self): + with force_color(True): + output = self.do_integration_test("ll\nx=42\n") + self.assertEqual(output["client"]["stderr"], "") + self.assertEqual(output["server"]["stderr"], "") + + self.assertEqual(output["server"]["stdout"], "Function returned: 42\n") + self.assertIn("\x1b", output["client"]["stdout"]) + self.assertNotIn("while x == 1", output["client"]["stdout"]) + self.assertIn("while x == 1", re.sub("\x1b[^m]*m", "", output["client"]["stdout"])) + if __name__ == "__main__": unittest.main() From 7797904c59ee2ca234ec350183bef6b443b9559e Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 02:04:43 -0400 Subject: [PATCH 3/9] Add a type annotation --- Lib/_pyrepl/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/_pyrepl/utils.py b/Lib/_pyrepl/utils.py index 7dc6f879a1e93a..38cf6b5a08e892 100644 --- a/Lib/_pyrepl/utils.py +++ b/Lib/_pyrepl/utils.py @@ -257,7 +257,7 @@ def disp_str( buffer: str, colors: list[ColorSpan] | None = None, start_index: int = 0, - force_color=False, + force_color: bool = False, ) -> tuple[CharBuffer, CharWidths]: r"""Decompose the input buffer into a printable variant with applied colors. From 7053a371d843a19807853311c15b7f7b4bc8b9bb Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 02:13:48 -0400 Subject: [PATCH 4/9] Print stderr if the integration test fails --- Lib/test/test_remote_pdb.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index d882fbb455a21a..9b054de131cdc6 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -1526,6 +1526,17 @@ def do_integration_test(self, client_stdin): stdout, stderr = attach_process.communicate(client_stdin, timeout=5) process.wait() + client_stdout = process.stdout.read() + client_stderr = process.stderr.read(), + + if process.returncode != 0: + print("server failed:") + print(stderr) + + if attach_process.returncode != 0: + print("client failed:") + print(client_stderr) + self.assertEqual(process.returncode, 0) self.assertEqual(attach_process.returncode, 0) @@ -1535,8 +1546,8 @@ def do_integration_test(self, client_stdin): "stderr": stderr, }, "server": { - "stdout": process.stdout.read(), - "stderr": process.stderr.read(), + "stdout": client_stdout, + "stderr": client_stderr, }, } From 0cdf9d3c5bad69a5d212a723b446840838c91c0a Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 02:22:01 -0400 Subject: [PATCH 5/9] Print stdout in addition to stderr on failure --- Lib/test/test_remote_pdb.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 9b054de131cdc6..1ec4da2b0a4512 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -1531,11 +1531,13 @@ def do_integration_test(self, client_stdin): if process.returncode != 0: print("server failed:") - print(stderr) + print(f"stdout:\n{stdout}") + print(f"stderr:\n{stderr}") if attach_process.returncode != 0: print("client failed:") - print(client_stderr) + print(f"stdout:\n{client_stdout}") + print(f"stderr:\n{client_stderr}") self.assertEqual(process.returncode, 0) self.assertEqual(attach_process.returncode, 0) From c92ee9d71f0e2776b86edef855d622d105ef39aa Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 02:30:34 -0400 Subject: [PATCH 6/9] More debugging... --- Lib/test/test_remote_pdb.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 1ec4da2b0a4512..7a7e4a4aee0633 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -1530,14 +1530,17 @@ def do_integration_test(self, client_stdin): client_stderr = process.stderr.read(), if process.returncode != 0: - print("server failed:") - print(f"stdout:\n{stdout}") - print(f"stderr:\n{stderr}") + print("server failed") if attach_process.returncode != 0: - print("client failed:") - print(f"stdout:\n{client_stdout}") - print(f"stderr:\n{client_stderr}") + print("client failed") + + if process.returncode != 0 or attach_process.returncode != 0: + print(f"server stdout:\n{stdout}") + print(f"server stderr:\n{stderr}") + + print(f"client stdout:\n{client_stdout}") + print(f"client stderr:\n{client_stderr}") self.assertEqual(process.returncode, 0) self.assertEqual(attach_process.returncode, 0) From c1b4dcee1e61f463fa9de7b32d47c43196c353eb Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 02:53:55 -0400 Subject: [PATCH 7/9] Don't run pdb -p in a child process Security restrictions on some platforms mean we can only attach to our immediate children. --- Lib/test/test_remote_pdb.py | 50 ++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 7a7e4a4aee0633..73a45bb138203a 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -13,7 +13,7 @@ import threading import unittest import unittest.mock -from contextlib import closing, contextmanager, redirect_stdout, ExitStack +from contextlib import closing, contextmanager, redirect_stdout, redirect_stderr, ExitStack from pathlib import Path from test.support import is_wasi, cpython_only, force_color, requires_subprocess, SHORT_TIMEOUT from test.support.os_helper import temp_dir, TESTFN, unlink @@ -1515,44 +1515,36 @@ def do_integration_test(self, client_stdin): conn, _ = self.sock.accept() conn.close() - attach_process = subprocess.Popen( - [sys.executable, "-m", "pdb", "-p", str(process.pid)], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - stdin=subprocess.PIPE, - text=True, - env=os.environ, - ) - stdout, stderr = attach_process.communicate(client_stdin, timeout=5) - process.wait() + client_stdin = io.StringIO(client_stdin) + client_stdout = io.StringIO() + client_stderr = io.StringIO() + + with ( + unittest.mock.patch("sys.stdin", client_stdin), + redirect_stdout(client_stdout), + redirect_stderr(client_stderr), + unittest.mock.patch("sys.argv", ["pdb", "-p", str(process.pid)]), + ): + pdb.main() - client_stdout = process.stdout.read() - client_stderr = process.stderr.read(), + process.wait() + server_stdout = process.stdout.read() + server_stderr = process.stderr.read() if process.returncode != 0: print("server failed") - - if attach_process.returncode != 0: - print("client failed") - - if process.returncode != 0 or attach_process.returncode != 0: - print(f"server stdout:\n{stdout}") - print(f"server stderr:\n{stderr}") - - print(f"client stdout:\n{client_stdout}") - print(f"client stderr:\n{client_stderr}") + print(f"server stdout:\n{server_stdout}") + print(f"server stderr:\n{server_stderr}") self.assertEqual(process.returncode, 0) - self.assertEqual(attach_process.returncode, 0) - return { "client": { - "stdout": stdout, - "stderr": stderr, + "stdout": client_stdout.getvalue(), + "stderr": client_stderr.getvalue(), }, "server": { - "stdout": client_stdout, - "stderr": client_stderr, + "stdout": server_stdout, + "stderr": server_stderr, }, } From 5a5e120e440c286ddc0a29cbb36652491f41a358 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 03:30:43 -0400 Subject: [PATCH 8/9] Clean up every fd we open --- Lib/test/test_remote_pdb.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 73a45bb138203a..3ceaf8bd581c97 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -1509,6 +1509,8 @@ def do_integration_test(self, client_stdin): stderr=subprocess.PIPE, text=True ) + self.addCleanup(process.stdout.close) + self.addCleanup(process.stderr.close) # Wait for the process to reach our attachment point self.sock.settimeout(10) @@ -1519,6 +1521,11 @@ def do_integration_test(self, client_stdin): client_stdout = io.StringIO() client_stderr = io.StringIO() + self.addCleanup(client_stdin.close) + self.addCleanup(client_stdout.close) + self.addCleanup(client_stderr.close) + self.addCleanup(process.wait) + with ( unittest.mock.patch("sys.stdin", client_stdin), redirect_stdout(client_stdout), From 89bdd52fb48ea4f5248406ece3944ae7fe28eb7a Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 May 2025 03:30:53 -0400 Subject: [PATCH 9/9] Skip the test if attaching fails --- Lib/test/test_remote_pdb.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 3ceaf8bd581c97..aef8a6b0129092 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -1532,7 +1532,10 @@ def do_integration_test(self, client_stdin): redirect_stderr(client_stderr), unittest.mock.patch("sys.argv", ["pdb", "-p", str(process.pid)]), ): - pdb.main() + try: + pdb.main() + except PermissionError: + self.skipTest("Insufficient permissions for remote execution") process.wait() server_stdout = process.stdout.read()