From 8d89bf4bfd57bca5e86eed541f4bce998668ae81 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 29 Apr 2025 21:28:16 -0400 Subject: [PATCH 01/18] Only allow KeyboardInterrupt at specific times Because our goal is to proxy interrupt signals to the remote process, it's important that we only accept those signals at moments when we're ready to handle them and send them on. In particular, we're prepared to handle them when reading user input for a prompt, and when waiting for the server to send a new prompt after our last command. We do not want to accept them at other times, like while we're in the middle of printing output to the screen, as otherwise a while True: "Hello" executed at the PDB REPL can't reliably be Ctrl-C'd: it's more likely that the signal will arrive while the client is printing than while it's waiting for the next line from the server, and if so nothing will be prepared to handle the `KeyboardInterrupt` and the client will exit. --- Lib/pdb.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index e2c7468c50c354..70fcd2d168772e 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -3009,12 +3009,53 @@ def readline_completion(self, completer): finally: readline.set_completer(old_completer) + if not hasattr(signal, "pthread_sigmask"): + # On Windows, we must drop signals arriving while we're ignoring them. + @contextmanager + def _block_sigint(self): + old_handler = signal.signal(signal.SIGINT, signal.SIG_IGN) + try: + yield + finally: + signal.signal(signal.SIGINT, old_handler) + + @contextmanager + def _handle_sigint(self, handler): + old_handler = signal.signal(signal.SIGINT, handler) + try: + yield + finally: + signal.signal(signal.SIGINT, old_handler) + else: + # On Unix, we can save them to be processed later. + @contextmanager + def _block_sigint(self): + signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) + try: + yield + finally: + signal.pthread_sigmask(signal.SIG_UNBLOCK, {signal.SIGINT}) + + @contextmanager + def _handle_sigint(self, handler): + old_handler = signal.signal(signal.SIGINT, handler) + try: + signal.pthread_sigmask(signal.SIG_UNBLOCK, {signal.SIGINT}) + yield + finally: + signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) + signal.signal(signal.SIGINT, old_handler) + def cmdloop(self): - with self.readline_completion(self.complete): + with ( + self._block_sigint(), + self.readline_completion(self.complete), + ): while not self.write_failed: try: - if not (payload_bytes := self.sockfile.readline()): - break + with self._handle_sigint(signal.default_int_handler): + if not (payload_bytes := self.sockfile.readline()): + break except KeyboardInterrupt: self.send_interrupt() continue @@ -3061,7 +3102,8 @@ def process_payload(self, payload): def prompt_for_reply(self, prompt): while True: try: - payload = {"reply": self.read_command(prompt)} + with self._handle_sigint(signal.default_int_handler): + payload = {"reply": self.read_command(prompt)} except EOFError: payload = {"signal": "EOF"} except KeyboardInterrupt: @@ -3101,7 +3143,8 @@ def complete(self, text, state): if self.write_failed: return None - payload = self.sockfile.readline() + with self._handle_sigint(signal.default_int_handler): + payload = self.sockfile.readline() if not payload: return None From f65f99ffb48cc472d193c21db019d7bf6b96edd6 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 29 Apr 2025 21:57:19 -0400 Subject: [PATCH 02/18] Have _PdbClient work with the socket directly Previously we worked with a file object created with `socket.makefile`, but on Windows the `readline()` method of a socket file can't be interrupted with Ctrl-C, and neither can `recv()` on a socket. This refactor lays the ground work for a solution this this problem. --- Lib/pdb.py | 26 ++++--- Lib/test/test_remote_pdb.py | 139 +++++++++--------------------------- 2 files changed, 48 insertions(+), 117 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 70fcd2d168772e..abb08301393a1e 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -2903,9 +2903,10 @@ def default(self, line): class _PdbClient: - def __init__(self, pid, sockfile, interrupt_script): + def __init__(self, pid, server_socket, interrupt_script): self.pid = pid - self.sockfile = sockfile + self.read_buf = b"" + self.server_socket = server_socket self.interrupt_script = interrupt_script self.pdb_instance = Pdb() self.pdb_commands = set() @@ -2947,8 +2948,7 @@ def _send(self, **kwargs): self._ensure_valid_message(kwargs) json_payload = json.dumps(kwargs) try: - self.sockfile.write(json_payload.encode() + b"\n") - self.sockfile.flush() + self.server_socket.sendall(json_payload.encode() + b"\n") except OSError: # This means that the client has abruptly disconnected, but we'll # handle that the next time we try to read from the client instead @@ -2957,6 +2957,15 @@ def _send(self, **kwargs): # return an empty string because the socket may be half-closed. self.write_failed = True + def _readline(self): + while b"\n" not in self.read_buf: + self.read_buf += self.server_socket.recv(16 * 1024) + if not self.read_buf: + return b"" + + ret, sep, self.read_buf = self.read_buf.partition(b"\n") + return ret + sep + def read_command(self, prompt): reply = input(prompt) @@ -3054,7 +3063,7 @@ def cmdloop(self): while not self.write_failed: try: with self._handle_sigint(signal.default_int_handler): - if not (payload_bytes := self.sockfile.readline()): + if not (payload_bytes := self._readline()): break except KeyboardInterrupt: self.send_interrupt() @@ -3144,7 +3153,7 @@ def complete(self, text, state): return None with self._handle_sigint(signal.default_int_handler): - payload = self.sockfile.readline() + payload = self._readline() if not payload: return None @@ -3211,9 +3220,6 @@ def attach(pid, commands=()): client_sock, _ = server.accept() with closing(client_sock): - sockfile = client_sock.makefile("rwb") - - with closing(sockfile): with tempfile.NamedTemporaryFile("w", delete_on_close=False) as interrupt_script: interrupt_script.write( 'import pdb, sys\n' @@ -3222,7 +3228,7 @@ def attach(pid, commands=()): ) interrupt_script.close() - _PdbClient(pid, sockfile, interrupt_script.name).cmdloop() + _PdbClient(pid, client_sock, interrupt_script.name).cmdloop() # Post-Mortem interface diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index e4c44c78d4a537..9dd7752ec248d8 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -12,7 +12,7 @@ import threading import unittest import unittest.mock -from contextlib import contextmanager, redirect_stdout, ExitStack +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.os_helper import temp_dir, TESTFN, unlink @@ -79,44 +79,6 @@ def get_output(self) -> List[dict]: return results -class MockDebuggerSocket: - """Mock file-like simulating a connection to a _RemotePdb instance""" - - def __init__(self, incoming): - self.incoming = iter(incoming) - self.outgoing = [] - self.buffered = bytearray() - - def write(self, data: bytes) -> None: - """Simulate write to socket.""" - self.buffered += data - - def flush(self) -> None: - """Ensure each line is valid JSON.""" - lines = self.buffered.splitlines(keepends=True) - self.buffered.clear() - for line in lines: - assert line.endswith(b"\n") - self.outgoing.append(json.loads(line)) - - def readline(self) -> bytes: - """Read a line from the prepared input queue.""" - # Anything written must be flushed before trying to read, - # since the read will be dependent upon the last write. - assert not self.buffered - try: - item = next(self.incoming) - if not isinstance(item, bytes): - item = json.dumps(item).encode() - return item + b"\n" - except StopIteration: - return b"" - - def close(self) -> None: - """No-op close implementation.""" - pass - - class PdbClientTestCase(unittest.TestCase): """Tests for the _PdbClient class.""" @@ -124,7 +86,7 @@ def do_test( self, *, incoming, - simulate_failure=None, + simulate_send_failure=False, expected_outgoing=None, expected_completions=None, expected_exception=None, @@ -142,16 +104,6 @@ def do_test( expected_state.setdefault("write_failed", False) messages = [m for source, m in incoming if source == "server"] prompts = [m["prompt"] for source, m in incoming if source == "user"] - sockfile = MockDebuggerSocket(messages) - stdout = io.StringIO() - - if simulate_failure: - sockfile.write = unittest.mock.Mock() - sockfile.flush = unittest.mock.Mock() - if simulate_failure == "write": - sockfile.write.side_effect = OSError("write failed") - elif simulate_failure == "flush": - sockfile.flush.side_effect = OSError("flush failed") input_iter = (m for source, m in incoming if source == "user") completions = [] @@ -181,6 +133,28 @@ def mock_input(prompt): return reply with ExitStack() as stack: + client_sock, server_sock = socket.socketpair() + stack.enter_context(closing(client_sock)) + stack.enter_context(closing(server_sock)) + + server_sock = unittest.mock.Mock(wraps=server_sock) + + client_sock.sendall( + b"".join( + (m if isinstance(m, bytes) else json.dumps(m).encode()) + b"\n" + for m in messages + ) + ) + client_sock.shutdown(socket.SHUT_WR) + + if simulate_send_failure: + server_sock.sendall = unittest.mock.Mock( + side_effect=OSError("sendall failed") + ) + client_sock.shutdown(socket.SHUT_RD) + + stdout = io.StringIO() + input_mock = stack.enter_context( unittest.mock.patch("pdb.input", side_effect=mock_input) ) @@ -188,7 +162,7 @@ def mock_input(prompt): client = _PdbClient( pid=0, - sockfile=sockfile, + server_socket=server_sock, interrupt_script="/a/b.py", ) @@ -199,13 +173,12 @@ def mock_input(prompt): client.cmdloop() - actual_outgoing = sockfile.outgoing - if simulate_failure: - actual_outgoing += [ - json.loads(msg.args[0]) for msg in sockfile.write.mock_calls - ] + sent_msgs = [msg.args[0] for msg in server_sock.sendall.mock_calls] + for msg in sent_msgs: + assert msg.endswith(b"\n") + actual_outgoing = [json.loads(msg) for msg in sent_msgs] - self.assertEqual(sockfile.outgoing, expected_outgoing) + self.assertEqual(actual_outgoing, expected_outgoing) self.assertEqual(completions, expected_completions) if expected_stdout_substring and not expected_stdout: self.assertIn(expected_stdout_substring, stdout.getvalue()) @@ -478,20 +451,7 @@ def test_write_failing(self): self.do_test( incoming=incoming, expected_outgoing=[{"signal": "INT"}], - simulate_failure="write", - expected_state={"write_failed": True}, - ) - - def test_flush_failing(self): - """Test terminating if flush fails due to a half closed socket.""" - incoming = [ - ("server", {"prompt": "(Pdb) ", "state": "pdb"}), - ("user", {"prompt": "(Pdb) ", "input": KeyboardInterrupt()}), - ] - self.do_test( - incoming=incoming, - expected_outgoing=[{"signal": "INT"}], - simulate_failure="flush", + simulate_send_failure=True, expected_state={"write_failed": True}, ) @@ -622,42 +582,7 @@ def test_write_failure_during_completion(self): }, {"reply": "xyz"}, ], - simulate_failure="write", - expected_completions=[], - expected_state={"state": "interact", "write_failed": True}, - ) - - def test_flush_failure_during_completion(self): - """Test failing to flush to the socket to request tab completions.""" - incoming = [ - ("server", {"prompt": ">>> ", "state": "interact"}), - ( - "user", - { - "prompt": ">>> ", - "completion_request": { - "line": "xy", - "begidx": 0, - "endidx": 2, - }, - "input": "xyz", - }, - ), - ] - self.do_test( - incoming=incoming, - expected_outgoing=[ - { - "complete": { - "text": "xy", - "line": "xy", - "begidx": 0, - "endidx": 2, - } - }, - {"reply": "xyz"}, - ], - simulate_failure="flush", + simulate_send_failure=True, expected_completions=[], expected_state={"state": "interact", "write_failed": True}, ) From ec9d3bf5600473a7cb5e0523d76e1d9f2ed6a7be Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 29 Apr 2025 22:52:44 -0400 Subject: [PATCH 03/18] Allow interrupting socket reads on Windows Since a socket read can't be directly interrupted on Windows, use the `selectors` module to watch for either new data to be read from that socket, or for a signal to arrive (leveraging `signal.set_wakeup_fd`). This allows us to watch for both types of events and handle whichever arrives first. --- Lib/pdb.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index abb08301393a1e..8d9df7623b4a3f 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -92,6 +92,7 @@ import itertools import traceback import linecache +import selectors import _colorize from contextlib import closing @@ -2906,6 +2907,8 @@ class _PdbClient: def __init__(self, pid, server_socket, interrupt_script): self.pid = pid self.read_buf = b"" + self.signal_read = None + self.signal_write = None self.server_socket = server_socket self.interrupt_script = interrupt_script self.pdb_instance = Pdb() @@ -2958,13 +2961,39 @@ def _send(self, **kwargs): self.write_failed = True def _readline(self): - while b"\n" not in self.read_buf: - self.read_buf += self.server_socket.recv(16 * 1024) - if not self.read_buf: - return b"" + # Wait for either a SIGINT or a line or EOF from the PDB server. + selector = selectors.DefaultSelector() + selector.register(self.signal_read, selectors.EVENT_READ) + selector.register(self.server_socket, selectors.EVENT_READ) + + old_wakeup_fd = signal.set_wakeup_fd( + self.signal_write.fileno(), + warn_on_full_buffer=False, + ) - ret, sep, self.read_buf = self.read_buf.partition(b"\n") - return ret + sep + got_sigint = False + def sigint_handler(*args, **kwargs): + nonlocal got_sigint + got_sigint = True + + old_handler = signal.signal(signal.SIGINT, sigint_handler) + try: + while b"\n" not in self.read_buf: + for key, _ in selector.select(): + if key.fileobj == self.signal_read: + self.signal_read.recv(1024) + if got_sigint: + raise KeyboardInterrupt + elif key.fileobj == self.server_socket: + self.read_buf += self.server_socket.recv(16 * 1024) + if not self.read_buf: + return b"" + + ret, sep, self.read_buf = self.read_buf.partition(b"\n") + return ret + sep + finally: + signal.set_wakeup_fd(old_wakeup_fd) + signal.signal(signal.SIGINT, old_handler) def read_command(self, prompt): reply = input(prompt) @@ -3055,9 +3084,21 @@ def _handle_sigint(self, handler): signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) signal.signal(signal.SIGINT, old_handler) + @contextmanager + def _signal_socket_pair(self): + self.signal_read, self.signal_write = socket.socketpair() + try: + with (closing(self.signal_read), closing(self.signal_write)): + self.signal_read.setblocking(False) + self.signal_write.setblocking(False) + yield + finally: + self.signal_read = self.signal_write = None + def cmdloop(self): with ( self._block_sigint(), + self._signal_socket_pair(), self.readline_completion(self.complete), ): while not self.write_failed: From ed664b839de66f0ba9b66c2093d2f668cb8df5f5 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Wed, 30 Apr 2025 14:55:18 -0400 Subject: [PATCH 04/18] Use a SIGINT to interrupt the remote on Unix This unfortunately cannot be applied to Windows, which has no way to trigger a SIGINT signal in a remote process without running code inside of that process itself. --- Lib/pdb.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 8d9df7623b4a3f..5cde150d6e7b8c 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -3122,11 +3122,20 @@ def cmdloop(self): self.process_payload(payload) def send_interrupt(self): - print( - "\n*** Program will stop at the next bytecode instruction." - " (Use 'cont' to resume)." - ) - sys.remote_exec(self.pid, self.interrupt_script) + if hasattr(signal, "pthread_kill"): + # On Unix, send a SIGINT to the remote process, which interrupts IO + # and makes it raise a KeyboardInterrupt on the main thread when + # PyErr_CheckSignals is called or the eval loop regains control. + os.kill(self.pid, signal.SIGINT) + else: + # On Windows, inject a remote script that calls Pdb.set_trace() + # when the eval loop regains control. This cannot interrupt IO, and + # also cannot interrupt statements executed at a PDB prompt. + print( + "\n*** Program will stop at the next bytecode instruction." + " (Use 'cont' to resume)." + ) + sys.remote_exec(self.pid, self.interrupt_script) def process_payload(self, payload): match payload: From aafa48ce50a74e500c89487b71435257be97fa5c Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Thu, 1 May 2025 18:26:29 -0400 Subject: [PATCH 05/18] Handle a ValueError for operations on a closed file It seems like this is raised explicitly in some cases. --- Lib/pdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 5cde150d6e7b8c..ae1bdf17e4b4a3 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -2678,7 +2678,7 @@ def _send(self, **kwargs): try: self._sockfile.write(json_payload.encode() + b"\n") self._sockfile.flush() - except OSError: + except (OSError, ValueError): # This means that the client has abruptly disconnected, but we'll # handle that the next time we try to read from the client instead # of trying to handle it from everywhere _send() may be called. From 42e7cec998f5167467fd317eeaaf7ca92f9c53d5 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Thu, 1 May 2025 18:27:18 -0400 Subject: [PATCH 06/18] Use a single complex signal handler for the PDB client --- Lib/pdb.py | 158 +++++++++++++++++++++++++++-------------------------- 1 file changed, 81 insertions(+), 77 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index ae1bdf17e4b4a3..e99ca102a8981d 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -2909,6 +2909,8 @@ def __init__(self, pid, server_socket, interrupt_script): self.read_buf = b"" self.signal_read = None self.signal_write = None + self.sigint_received = False + self.raise_on_sigint = False self.server_socket = server_socket self.interrupt_script = interrupt_script self.pdb_instance = Pdb() @@ -2961,42 +2963,39 @@ def _send(self, **kwargs): self.write_failed = True def _readline(self): + if self.sigint_received: + # There's a pending unhandled SIGINT. Handle it now. + self.sigint_received = False + raise KeyboardInterrupt + # Wait for either a SIGINT or a line or EOF from the PDB server. selector = selectors.DefaultSelector() selector.register(self.signal_read, selectors.EVENT_READ) selector.register(self.server_socket, selectors.EVENT_READ) - old_wakeup_fd = signal.set_wakeup_fd( - self.signal_write.fileno(), - warn_on_full_buffer=False, - ) - - got_sigint = False - def sigint_handler(*args, **kwargs): - nonlocal got_sigint - got_sigint = True + while b"\n" not in self.read_buf: + for key, _ in selector.select(): + if key.fileobj == self.signal_read: + self.signal_read.recv(1024) + if self.sigint_received: + # If not, we're reading wakeup events for sigints that + # we've previously handled, and can ignore them. + self.sigint_received = False + raise KeyboardInterrupt + elif key.fileobj == self.server_socket: + data = self.server_socket.recv(16 * 1024) + self.read_buf += data + if not data and b"\n" not in self.read_buf: + # EOF without a full final line. Drop the partial line. + self.read_buf = b"" + return b"" - old_handler = signal.signal(signal.SIGINT, sigint_handler) - try: - while b"\n" not in self.read_buf: - for key, _ in selector.select(): - if key.fileobj == self.signal_read: - self.signal_read.recv(1024) - if got_sigint: - raise KeyboardInterrupt - elif key.fileobj == self.server_socket: - self.read_buf += self.server_socket.recv(16 * 1024) - if not self.read_buf: - return b"" - - ret, sep, self.read_buf = self.read_buf.partition(b"\n") - return ret + sep - finally: - signal.set_wakeup_fd(old_wakeup_fd) - signal.signal(signal.SIGINT, old_handler) + ret, sep, self.read_buf = self.read_buf.partition(b"\n") + return ret + sep def read_command(self, prompt): - reply = input(prompt) + with self._sigint_raises_keyboard_interrupt(): + reply = input(prompt) if self.state == "dumb": # No logic applied whatsoever, just pass the raw reply back. @@ -3022,7 +3021,8 @@ def read_command(self, prompt): # Otherwise, valid first line of a multi-line statement continue_prompt = "...".ljust(len(prompt)) while codeop.compile_command(reply, "", "single") is None: - reply += "\n" + input(continue_prompt) + with self._sigint_raises_keyboard_interrupt(): + reply += "\n" + input(continue_prompt) return prefix + reply @@ -3047,65 +3047,71 @@ def readline_completion(self, completer): finally: readline.set_completer(old_completer) - if not hasattr(signal, "pthread_sigmask"): - # On Windows, we must drop signals arriving while we're ignoring them. - @contextmanager - def _block_sigint(self): - old_handler = signal.signal(signal.SIGINT, signal.SIG_IGN) - try: - yield - finally: - signal.signal(signal.SIGINT, old_handler) + @contextmanager + def _sigint_handler(self): + # Signal handling strategy: + # - When we call input() we want a SIGINT to raise KeyboardInterrupt + # - Otherwise we want to write to the wakeup FD and set a flag. + # We'll break out of select() when the wakeup FD is written to, + # and we'll check the flag whenever we're about to accept input. + def handler(signum, frame): + self.sigint_received = True + if self.raise_on_sigint: + # One-shot; don't raise again until the flag is set again. + self.raise_on_sigint = False + self.sigint_received = False + raise KeyboardInterrupt + + sentinel = object() + old_handler = sentinel + old_wakeup_fd = sentinel - @contextmanager - def _handle_sigint(self, handler): - old_handler = signal.signal(signal.SIGINT, handler) - try: - yield - finally: - signal.signal(signal.SIGINT, old_handler) - else: - # On Unix, we can save them to be processed later. - @contextmanager - def _block_sigint(self): - signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) - try: - yield - finally: - signal.pthread_sigmask(signal.SIG_UNBLOCK, {signal.SIGINT}) + self.signal_read, self.signal_write = socket.socketpair() + with (closing(self.signal_read), closing(self.signal_write)): + self.signal_read.setblocking(False) + self.signal_write.setblocking(False) - @contextmanager - def _handle_sigint(self, handler): - old_handler = signal.signal(signal.SIGINT, handler) try: - signal.pthread_sigmask(signal.SIG_UNBLOCK, {signal.SIGINT}) - yield + old_handler = signal.signal(signal.SIGINT, handler) + + try: + old_wakeup_fd = signal.set_wakeup_fd( + self.signal_write.fileno(), + warn_on_full_buffer=False, + ) + yield + finally: + # Restore the old wakeup fd if we installed a new one + if old_wakeup_fd is not sentinel: + signal.set_wakeup_fd(old_wakeup_fd) + self.signal_read = self.signal_write = None finally: - signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGINT}) - signal.signal(signal.SIGINT, old_handler) + if old_handler is not sentinel: + # Restore the old handler if we installed a new one + signal.signal(signal.SIGINT, old_handler) @contextmanager - def _signal_socket_pair(self): - self.signal_read, self.signal_write = socket.socketpair() + def _sigint_raises_keyboard_interrupt(self): + if self.sigint_received: + # There's a pending unhandled SIGINT. Handle it now. + self.sigint_received = False + raise KeyboardInterrupt + try: - with (closing(self.signal_read), closing(self.signal_write)): - self.signal_read.setblocking(False) - self.signal_write.setblocking(False) - yield + self.raise_on_sigint = True + yield finally: - self.signal_read = self.signal_write = None + self.raise_on_sigint = False def cmdloop(self): with ( - self._block_sigint(), - self._signal_socket_pair(), + self._sigint_handler(), self.readline_completion(self.complete), ): while not self.write_failed: try: - with self._handle_sigint(signal.default_int_handler): - if not (payload_bytes := self._readline()): - break + if not (payload_bytes := self._readline()): + break except KeyboardInterrupt: self.send_interrupt() continue @@ -3161,8 +3167,7 @@ def process_payload(self, payload): def prompt_for_reply(self, prompt): while True: try: - with self._handle_sigint(signal.default_int_handler): - payload = {"reply": self.read_command(prompt)} + payload = {"reply": self.read_command(prompt)} except EOFError: payload = {"signal": "EOF"} except KeyboardInterrupt: @@ -3202,8 +3207,7 @@ def complete(self, text, state): if self.write_failed: return None - with self._handle_sigint(signal.default_int_handler): - payload = self._readline() + payload = self._readline() if not payload: return None From 02da647e38a2060905ed044b02b1c13622755036 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Thu, 1 May 2025 18:32:53 -0400 Subject: [PATCH 07/18] Add a news entry --- .../next/Library/2025-05-01-18-32-44.gh-issue-133223.KE_T5f.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-05-01-18-32-44.gh-issue-133223.KE_T5f.rst diff --git a/Misc/NEWS.d/next/Library/2025-05-01-18-32-44.gh-issue-133223.KE_T5f.rst b/Misc/NEWS.d/next/Library/2025-05-01-18-32-44.gh-issue-133223.KE_T5f.rst new file mode 100644 index 00000000000000..41d2f87a79056b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-05-01-18-32-44.gh-issue-133223.KE_T5f.rst @@ -0,0 +1,2 @@ +When PDB is attached to a remote process, do a better job of intercepting +Ctrl+C and forwarding it to the remote process. From 5abd63a4e48cbb562ec23363beafc8c628466c49 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Thu, 1 May 2025 20:43:07 -0400 Subject: [PATCH 08/18] Update the comment to explain why we catch two exception types --- Lib/pdb.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index e99ca102a8981d..639d59c770b6ce 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -2679,8 +2679,9 @@ def _send(self, **kwargs): self._sockfile.write(json_payload.encode() + b"\n") self._sockfile.flush() except (OSError, ValueError): - # This means that the client has abruptly disconnected, but we'll - # handle that the next time we try to read from the client instead + # We get an OSError if the network connection has dropped, and a + # ValueError if detach() if the sockfile has been closed. We'll + # handle this the next time we try to read from the client instead # of trying to handle it from everywhere _send() may be called. # Track this with a flag rather than assuming readline() will ever # return an empty string because the socket may be half-closed. From c9c5bf27231180c0964ad416d20d381fffe010bc Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Thu, 1 May 2025 20:43:54 -0400 Subject: [PATCH 09/18] Swap signal_read/signal_write resetting to the outer finally block --- Lib/pdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 639d59c770b6ce..5d2891775fa577 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -3085,8 +3085,8 @@ def handler(signum, frame): # Restore the old wakeup fd if we installed a new one if old_wakeup_fd is not sentinel: signal.set_wakeup_fd(old_wakeup_fd) - self.signal_read = self.signal_write = None finally: + self.signal_read = self.signal_write = None if old_handler is not sentinel: # Restore the old handler if we installed a new one signal.signal(signal.SIGINT, old_handler) From 8fa88a5412c9353af3c23963a32ce6092f870981 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Thu, 1 May 2025 20:47:14 -0400 Subject: [PATCH 10/18] Use os.kill() on every platform but Windows --- Lib/pdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 5d2891775fa577..624fca5255276a 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -3129,7 +3129,7 @@ def cmdloop(self): self.process_payload(payload) def send_interrupt(self): - if hasattr(signal, "pthread_kill"): + if sys.platform != "win32": # On Unix, send a SIGINT to the remote process, which interrupts IO # and makes it raise a KeyboardInterrupt on the main thread when # PyErr_CheckSignals is called or the eval loop regains control. From 4c0b431a25bcd902b7bf5316ba7474e591d6f9ef Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Fri, 2 May 2025 11:21:35 -0400 Subject: [PATCH 11/18] Use `ExitStack` to reduce nesting in `attach` --- Lib/pdb.py | 69 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 624fca5255276a..6096b27abc7408 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -95,6 +95,7 @@ import selectors import _colorize +from contextlib import ExitStack from contextlib import closing from contextlib import contextmanager from rlcompleter import Completer @@ -3250,40 +3251,50 @@ def _connect(host, port, frame, commands, version): def attach(pid, commands=()): """Attach to a running process with the given PID.""" - with closing(socket.create_server(("localhost", 0))) as server: + with ExitStack() as stack: + server = stack.enter_context( + closing(socket.create_server(("localhost", 0))) + ) + port = server.getsockname()[1] - with tempfile.NamedTemporaryFile("w", delete_on_close=False) as connect_script: - connect_script.write( - textwrap.dedent( - f""" - import pdb, sys - pdb._connect( - host="localhost", - port={port}, - frame=sys._getframe(1), - commands={json.dumps("\n".join(commands))}, - version={_PdbServer.protocol_version()}, - ) - """ + connect_script = stack.enter_context( + tempfile.NamedTemporaryFile("w", delete_on_close=False) + ) + + connect_script.write( + textwrap.dedent( + f""" + import pdb, sys + pdb._connect( + host="localhost", + port={port}, + frame=sys._getframe(1), + commands={json.dumps("\n".join(commands))}, + version={_PdbServer.protocol_version()}, ) + """ ) - connect_script.close() - sys.remote_exec(pid, connect_script.name) - - # TODO Add a timeout? Or don't bother since the user can ^C? - client_sock, _ = server.accept() - - with closing(client_sock): - with tempfile.NamedTemporaryFile("w", delete_on_close=False) as interrupt_script: - interrupt_script.write( - 'import pdb, sys\n' - 'if inst := pdb.Pdb._last_pdb_instance:\n' - ' inst.set_trace(sys._getframe(1))\n' - ) - interrupt_script.close() + ) + connect_script.close() + sys.remote_exec(pid, connect_script.name) + + # TODO Add a timeout? Or don't bother since the user can ^C? + client_sock, _ = server.accept() + + stack.enter_context(closing(client_sock)) + + interrupt_script = stack.enter_context( + tempfile.NamedTemporaryFile("w", delete_on_close=False) + ) + interrupt_script.write( + 'import pdb, sys\n' + 'if inst := pdb.Pdb._last_pdb_instance:\n' + ' inst.set_trace(sys._getframe(1))\n' + ) + interrupt_script.close() - _PdbClient(pid, client_sock, interrupt_script.name).cmdloop() + _PdbClient(pid, client_sock, interrupt_script.name).cmdloop() # Post-Mortem interface From 5993e06c3437cd9ae78d824cfcc22ca3cd2c5387 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Fri, 2 May 2025 14:07:15 -0400 Subject: [PATCH 12/18] Use a thread to manage interrupts on Windows --- Lib/pdb.py | 79 ++++++++++++++++++++++++++----------- Lib/test/test_remote_pdb.py | 2 +- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 6096b27abc7408..fbfdd5360cbdfe 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -77,6 +77,7 @@ import json import token import types +import atexit import codeop import pprint import signal @@ -93,6 +94,7 @@ import traceback import linecache import selectors +import threading import _colorize from contextlib import ExitStack @@ -2906,7 +2908,7 @@ def default(self, line): class _PdbClient: - def __init__(self, pid, server_socket, interrupt_script): + def __init__(self, pid, server_socket, interrupt_sock): self.pid = pid self.read_buf = b"" self.signal_read = None @@ -2914,7 +2916,7 @@ def __init__(self, pid, server_socket, interrupt_script): self.sigint_received = False self.raise_on_sigint = False self.server_socket = server_socket - self.interrupt_script = interrupt_script + self.interrupt_sock = interrupt_sock self.pdb_instance = Pdb() self.pdb_commands = set() self.completion_matches = [] @@ -3136,14 +3138,11 @@ def send_interrupt(self): # PyErr_CheckSignals is called or the eval loop regains control. os.kill(self.pid, signal.SIGINT) else: - # On Windows, inject a remote script that calls Pdb.set_trace() - # when the eval loop regains control. This cannot interrupt IO, and - # also cannot interrupt statements executed at a PDB prompt. - print( - "\n*** Program will stop at the next bytecode instruction." - " (Use 'cont' to resume)." - ) - sys.remote_exec(self.pid, self.interrupt_script) + # On Windows, write to a socket that the PDB server listens on. + # This triggers the remote to raise a SIGINT for itself. We do this + # because Windows doesn't allow triggering SIGINT remotely. + # See https://stackoverflow.com/a/35792192 for many more details. + self.interrupt_sock.sendall(signal.SIGINT.to_bytes()) def process_payload(self, payload): match payload: @@ -3226,6 +3225,41 @@ def complete(self, text, state): return None +def _start_interrupt_listener(host, port): + def sigint_listener(host, port): + with closing( + socket.create_connection((host, port), timeout=5) + ) as sock: + # Check if the interpreter is finalizing every quarter of a second. + # Clean up and exit if so. + sock.settimeout(0.25) + sock.shutdown(socket.SHUT_WR) + while not shut_down.is_set(): + try: + data = sock.recv(1024) + except socket.timeout: + continue + if data == b"": + return # EOF + signal.raise_signal(signal.SIGINT) + + def stop_thread(): + shut_down.set() + thread.join() + + # Use a daemon thread so that we don't detach until after all non-daemon + # threads are done. Use an atexit handler to stop gracefully at that point, + # so that our thread is stopped before the interpreter is torn down. + shut_down = threading.Event() + thread = threading.Thread( + target=sigint_listener, + args=(host, port), + daemon=True, + ) + atexit.register(stop_thread) + thread.start() + + def _connect(host, port, frame, commands, version): with closing(socket.create_connection((host, port))) as conn: sockfile = conn.makefile("rwb") @@ -3255,9 +3289,14 @@ def attach(pid, commands=()): server = stack.enter_context( closing(socket.create_server(("localhost", 0))) ) - port = server.getsockname()[1] + if sys.platform == "win32": + commands = [ + f"__import__('pdb')._start_interrupt_listener('localhost', {port})", + *commands, + ] + connect_script = stack.enter_context( tempfile.NamedTemporaryFile("w", delete_on_close=False) ) @@ -3281,20 +3320,16 @@ def attach(pid, commands=()): # TODO Add a timeout? Or don't bother since the user can ^C? client_sock, _ = server.accept() - stack.enter_context(closing(client_sock)) - interrupt_script = stack.enter_context( - tempfile.NamedTemporaryFile("w", delete_on_close=False) - ) - interrupt_script.write( - 'import pdb, sys\n' - 'if inst := pdb.Pdb._last_pdb_instance:\n' - ' inst.set_trace(sys._getframe(1))\n' - ) - interrupt_script.close() + if sys.platform == "win32": + interrupt_sock, _ = server.accept() + stack.enter_context(closing(interrupt_sock)) + interrupt_sock.setblocking(False) + else: + interrupt_sock = None - _PdbClient(pid, client_sock, interrupt_script.name).cmdloop() + _PdbClient(pid, client_sock, interrupt_sock).cmdloop() # Post-Mortem interface diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 9dd7752ec248d8..5665e2b3796989 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -163,7 +163,7 @@ def mock_input(prompt): client = _PdbClient( pid=0, server_socket=server_sock, - interrupt_script="/a/b.py", + interrupt_sock=unittest.mock.Mock(spec=socket.socket), ) if expected_exception is not None: From dd7c9b1d877f2c8eaf6dced33b0d7757034c4a2a Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Sun, 4 May 2025 14:35:41 -0400 Subject: [PATCH 13/18] Use the signal handling thread approach on all platforms Let the caller of `_connect` tell us whether to start the thread. --- Lib/pdb.py | 125 ++++++++++++++++++------------------ Lib/test/test_remote_pdb.py | 26 +++----- 2 files changed, 72 insertions(+), 79 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index fbfdd5360cbdfe..11e5766ac6da0b 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -2614,12 +2614,21 @@ async def set_trace_async(*, header=None, commands=None): # Remote PDB class _PdbServer(Pdb): - def __init__(self, sockfile, owns_sockfile=True, **kwargs): + def __init__( + self, + sockfile, + signal_server=None, + owns_sockfile=True, + **kwargs, + ): self._owns_sockfile = owns_sockfile self._interact_state = None self._sockfile = sockfile self._command_name_cache = [] self._write_failed = False + if signal_server: + # Only started by the top level _PdbServer, not recursive ones. + self._start_signal_listener(signal_server) super().__init__(**kwargs) @staticmethod @@ -2675,6 +2684,39 @@ def _ensure_valid_message(self, msg): f"PDB message doesn't follow the schema! {msg}" ) + @classmethod + def _start_signal_listener(cls, address): + def listener(sock): + with closing(sock): + # Check if the interpreter is finalizing every quarter of a second. + # Clean up and exit if so. + sock.settimeout(0.25) + sock.shutdown(socket.SHUT_WR) + while not shut_down.is_set(): + try: + data = sock.recv(1024) + except socket.timeout: + continue + if data == b"": + return # EOF + signal.raise_signal(signal.SIGINT) + + def stop_thread(): + shut_down.set() + thread.join() + + # Use a daemon thread so that we don't detach until after all non-daemon + # threads are done. Use an atexit handler to stop gracefully at that point, + # so that our thread is stopped before the interpreter is torn down. + shut_down = threading.Event() + thread = threading.Thread( + target=listener, + args=[socket.create_connection(address, timeout=5)], + daemon=True, + ) + atexit.register(stop_thread) + thread.start() + def _send(self, **kwargs): self._ensure_valid_message(kwargs) json_payload = json.dumps(kwargs) @@ -3132,17 +3174,13 @@ def cmdloop(self): self.process_payload(payload) def send_interrupt(self): - if sys.platform != "win32": - # On Unix, send a SIGINT to the remote process, which interrupts IO - # and makes it raise a KeyboardInterrupt on the main thread when - # PyErr_CheckSignals is called or the eval loop regains control. - os.kill(self.pid, signal.SIGINT) - else: - # On Windows, write to a socket that the PDB server listens on. - # This triggers the remote to raise a SIGINT for itself. We do this - # because Windows doesn't allow triggering SIGINT remotely. - # See https://stackoverflow.com/a/35792192 for many more details. - self.interrupt_sock.sendall(signal.SIGINT.to_bytes()) + # Write to a socket that the PDB server listens on. This triggers + # the remote to raise a SIGINT for itself. We do this because + # Windows doesn't allow triggering SIGINT remotely. + # See https://stackoverflow.com/a/35792192 for many more details. + # We could directly send SIGINT to the remote process on Unix, but + # doing the same thing on all platforms simplifies maintenance. + self.interrupt_sock.sendall(signal.SIGINT.to_bytes()) def process_payload(self, payload): match payload: @@ -3225,46 +3263,19 @@ def complete(self, text, state): return None -def _start_interrupt_listener(host, port): - def sigint_listener(host, port): - with closing( - socket.create_connection((host, port), timeout=5) - ) as sock: - # Check if the interpreter is finalizing every quarter of a second. - # Clean up and exit if so. - sock.settimeout(0.25) - sock.shutdown(socket.SHUT_WR) - while not shut_down.is_set(): - try: - data = sock.recv(1024) - except socket.timeout: - continue - if data == b"": - return # EOF - signal.raise_signal(signal.SIGINT) - - def stop_thread(): - shut_down.set() - thread.join() - - # Use a daemon thread so that we don't detach until after all non-daemon - # threads are done. Use an atexit handler to stop gracefully at that point, - # so that our thread is stopped before the interpreter is torn down. - shut_down = threading.Event() - thread = threading.Thread( - target=sigint_listener, - args=(host, port), - daemon=True, - ) - atexit.register(stop_thread) - thread.start() - - -def _connect(host, port, frame, commands, version): +def _connect(host, port, frame, commands, version, signal_raising_thread): with closing(socket.create_connection((host, port))) as conn: sockfile = conn.makefile("rwb") - remote_pdb = _PdbServer(sockfile) + # Starting a signal raising thread is optional to allow us the flexibility + # to switch to sending signals directly on Unix platforms in the future + # without breaking backwards compatibility. This also makes tests simpler. + if signal_raising_thread: + signal_server = (host, port) + else: + signal_server = None + + remote_pdb = _PdbServer(sockfile, signal_server=signal_server) weakref.finalize(remote_pdb, sockfile.close) if Pdb._last_pdb_instance is not None: @@ -3291,12 +3302,6 @@ def attach(pid, commands=()): ) port = server.getsockname()[1] - if sys.platform == "win32": - commands = [ - f"__import__('pdb')._start_interrupt_listener('localhost', {port})", - *commands, - ] - connect_script = stack.enter_context( tempfile.NamedTemporaryFile("w", delete_on_close=False) ) @@ -3311,6 +3316,7 @@ def attach(pid, commands=()): frame=sys._getframe(1), commands={json.dumps("\n".join(commands))}, version={_PdbServer.protocol_version()}, + signal_raising_thread=True, ) """ ) @@ -3322,12 +3328,9 @@ def attach(pid, commands=()): client_sock, _ = server.accept() stack.enter_context(closing(client_sock)) - if sys.platform == "win32": - interrupt_sock, _ = server.accept() - stack.enter_context(closing(interrupt_sock)) - interrupt_sock.setblocking(False) - else: - interrupt_sock = None + interrupt_sock, _ = server.accept() + stack.enter_context(closing(interrupt_sock)) + interrupt_sock.setblocking(False) _PdbClient(pid, client_sock, interrupt_sock).cmdloop() diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 5665e2b3796989..417e9726d670ba 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -919,6 +919,7 @@ def dummy_function(): frame=frame, commands="", version=pdb._PdbServer.protocol_version(), + signal_raising_thread=False, ) return x # This line won't be reached in debugging @@ -976,23 +977,6 @@ def _send_command(self, client_file, command): client_file.write(json.dumps({"reply": command}).encode() + b"\n") client_file.flush() - def _send_interrupt(self, pid): - """Helper to send an interrupt signal to the debugger.""" - # with tempfile.NamedTemporaryFile("w", delete_on_close=False) as interrupt_script: - interrupt_script = TESTFN + "_interrupt_script.py" - with open(interrupt_script, 'w') as f: - f.write( - 'import pdb, sys\n' - 'print("Hello, world!")\n' - 'if inst := pdb.Pdb._last_pdb_instance:\n' - ' inst.set_trace(sys._getframe(1))\n' - ) - self.addCleanup(unlink, interrupt_script) - try: - sys.remote_exec(pid, interrupt_script) - except PermissionError: - self.skipTest("Insufficient permissions to execute code in remote process") - def test_connect_and_basic_commands(self): """Test connecting to a remote debugger and sending basic commands.""" self._create_script() @@ -1105,6 +1089,7 @@ def bar(): frame=frame, commands="", version=pdb._PdbServer.protocol_version(), + signal_raising_thread=True, ) print("Connected to debugger") iterations = 50 @@ -1120,6 +1105,10 @@ def bar(): self._create_script(script=script) process, client_file = self._connect_and_get_client_file() + # Accept a 2nd connection from the subprocess to tell it about signals + signal_sock, _ = self.server_sock.accept() + self.addCleanup(signal_sock.close) + with kill_on_error(process): # Skip initial messages until we get to the prompt self._read_until_prompt(client_file) @@ -1135,7 +1124,7 @@ def bar(): break # Inject a script to interrupt the running process - self._send_interrupt(process.pid) + signal_sock.sendall(signal.SIGINT.to_bytes()) messages = self._read_until_prompt(client_file) # Verify we got the keyboard interrupt message. @@ -1191,6 +1180,7 @@ def run_test(): frame=frame, commands="", version=fake_version, + signal_raising_thread=False, ) # This should print if the debugger detaches correctly From e2391b0fb3e791517561e1e4966c98862c8734ec Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Sun, 4 May 2025 14:38:20 -0400 Subject: [PATCH 14/18] Make all _connect arguments keyword-only This gives us greater flexibility to reorder things or add things in the future without breaking backwards compatibility. --- Lib/pdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 11e5766ac6da0b..66067e3ed81bac 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -3263,7 +3263,7 @@ def complete(self, text, state): return None -def _connect(host, port, frame, commands, version, signal_raising_thread): +def _connect(*, host, port, frame, commands, version, signal_raising_thread): with closing(socket.create_connection((host, port))) as conn: sockfile = conn.makefile("rwb") From 14aa8e7ce62893951e1476a500c79f55d3e5541d Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Sun, 4 May 2025 16:30:44 -0400 Subject: [PATCH 15/18] One line for contextlib imports --- Lib/pdb.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index e59825663784ae..556247bea6a1ae 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -97,9 +97,7 @@ import threading import _colorize -from contextlib import ExitStack -from contextlib import closing -from contextlib import contextmanager +from contextlib import ExitStack, closing, contextmanager from rlcompleter import Completer from types import CodeType from warnings import deprecated From b26ed5c039cb71422ffca70da91a4e2e051aa7ac Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Sun, 4 May 2025 17:05:48 -0400 Subject: [PATCH 16/18] Add some tests for handling SIGINT in the PDB client --- Lib/test/test_remote_pdb.py | 72 ++++++++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index 3c7d04eb3a4a32..bef486052c9961 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -87,7 +87,9 @@ def do_test( *, incoming, simulate_send_failure=False, + simulate_sigint_during_stdout_write=False, expected_outgoing=None, + expected_outgoing_signals=None, expected_completions=None, expected_exception=None, expected_stdout="", @@ -96,6 +98,8 @@ def do_test( ): if expected_outgoing is None: expected_outgoing = [] + if expected_outgoing_signals is None: + expected_outgoing_signals = [] if expected_completions is None: expected_completions = [] if expected_state is None: @@ -130,7 +134,9 @@ def mock_input(prompt): reply = message["input"] if isinstance(reply, BaseException): raise reply - return reply + if isinstance(reply, str): + return reply + return reply() with ExitStack() as stack: client_sock, server_sock = socket.socketpair() @@ -155,15 +161,25 @@ def mock_input(prompt): stdout = io.StringIO() + if simulate_sigint_during_stdout_write: + orig_stdout_write = stdout.write + + def sigint_stdout_write(s): + signal.raise_signal(signal.SIGINT) + return orig_stdout_write(s) + + stdout.write = sigint_stdout_write + input_mock = stack.enter_context( unittest.mock.patch("pdb.input", side_effect=mock_input) ) stack.enter_context(redirect_stdout(stdout)) + interrupt_sock = unittest.mock.Mock(spec=socket.socket) client = _PdbClient( pid=0, server_socket=server_sock, - interrupt_sock=unittest.mock.Mock(spec=socket.socket), + interrupt_sock=interrupt_sock, ) if expected_exception is not None: @@ -188,6 +204,12 @@ def mock_input(prompt): actual_state = {k: getattr(client, k) for k in expected_state} self.assertEqual(actual_state, expected_state) + outgoing_signals = [ + signal.Signals(int.from_bytes(call.args[0])) + for call in interrupt_sock.sendall.call_args_list + ] + self.assertEqual(outgoing_signals, expected_outgoing_signals) + def test_remote_immediately_closing_the_connection(self): """Test the behavior when the remote closes the connection immediately.""" incoming = [] @@ -382,11 +404,17 @@ def test_handling_unrecognized_prompt_type(self): expected_state={"state": "dumb"}, ) - def test_keyboard_interrupt_at_prompt(self): - """Test signaling when a prompt gets a KeyboardInterrupt.""" + def test_sigint_at_prompt(self): + """Test signaling when a prompt gets interrupted.""" incoming = [ ("server", {"prompt": "(Pdb) ", "state": "pdb"}), - ("user", {"prompt": "(Pdb) ", "input": KeyboardInterrupt()}), + ( + "user", + { + "prompt": "(Pdb) ", + "input": lambda: signal.raise_signal(signal.SIGINT), + }, + ), ] self.do_test( incoming=incoming, @@ -396,6 +424,40 @@ def test_keyboard_interrupt_at_prompt(self): expected_state={"state": "pdb"}, ) + def test_sigint_at_continuation_prompt(self): + """Test signaling when a continuation prompt gets interrupted.""" + incoming = [ + ("server", {"prompt": "(Pdb) ", "state": "pdb"}), + ("user", {"prompt": "(Pdb) ", "input": "if True:"}), + ( + "user", + { + "prompt": "... ", + "input": lambda: signal.raise_signal(signal.SIGINT), + }, + ), + ] + self.do_test( + incoming=incoming, + expected_outgoing=[ + {"signal": "INT"}, + ], + expected_state={"state": "pdb"}, + ) + + def test_sigint_when_writing(self): + """Test siginaling when sys.stdout.write() gets interrupted.""" + incoming = [ + ("server", {"message": "Some message or other\n", "type": "info"}), + ] + self.do_test( + incoming=incoming, + simulate_sigint_during_stdout_write=True, + expected_outgoing=[], + expected_outgoing_signals=[signal.SIGINT], + expected_stdout="Some message or other\n", + ) + def test_eof_at_prompt(self): """Test signaling when a prompt gets an EOFError.""" incoming = [ From a860087d9c41980d49ac3229708e8a84c0712c9b Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Sun, 4 May 2025 17:39:37 -0400 Subject: [PATCH 17/18] Switch back to os.kill() on Unix --- Lib/pdb.py | 36 ++++++++++++++++++----------- Lib/test/test_remote_pdb.py | 46 ++++++++++++++++++++++++++----------- 2 files changed, 55 insertions(+), 27 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 556247bea6a1ae..5417ba36f48de1 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -3196,13 +3196,17 @@ def cmdloop(self): self.process_payload(payload) def send_interrupt(self): - # Write to a socket that the PDB server listens on. This triggers - # the remote to raise a SIGINT for itself. We do this because - # Windows doesn't allow triggering SIGINT remotely. - # See https://stackoverflow.com/a/35792192 for many more details. - # We could directly send SIGINT to the remote process on Unix, but - # doing the same thing on all platforms simplifies maintenance. - self.interrupt_sock.sendall(signal.SIGINT.to_bytes()) + if self.interrupt_sock is not None: + # Write to a socket that the PDB server listens on. This triggers + # the remote to raise a SIGINT for itself. We do this because + # Windows doesn't allow triggering SIGINT remotely. + # See https://stackoverflow.com/a/35792192 for many more details. + self.interrupt_sock.sendall(signal.SIGINT.to_bytes()) + else: + # On Unix we can just send a SIGINT to the remote process. + # This is preferable to using the signal thread approach that we + # use on Windows because it can interrupt IO in the main thread. + os.kill(self.pid, signal.SIGINT) def process_payload(self, payload): match payload: @@ -3293,9 +3297,8 @@ def _connect(*, host, port, frame, commands, version, signal_raising_thread): with closing(socket.create_connection((host, port))) as conn: sockfile = conn.makefile("rwb") - # Starting a signal raising thread is optional to allow us the flexibility - # to switch to sending signals directly on Unix platforms in the future - # without breaking backwards compatibility. This also makes tests simpler. + # The client requests this thread on Windows but not on Unix. + # Most tests don't request this thread, to keep them simpler. if signal_raising_thread: signal_server = (host, port) else: @@ -3332,6 +3335,8 @@ def attach(pid, commands=()): tempfile.NamedTemporaryFile("w", delete_on_close=False) ) + use_signal_thread = sys.platform == "win32" + connect_script.write( textwrap.dedent( f""" @@ -3342,7 +3347,7 @@ def attach(pid, commands=()): frame=sys._getframe(1), commands={json.dumps("\n".join(commands))}, version={_PdbServer.protocol_version()}, - signal_raising_thread=True, + signal_raising_thread={use_signal_thread!r}, ) """ ) @@ -3354,9 +3359,12 @@ def attach(pid, commands=()): client_sock, _ = server.accept() stack.enter_context(closing(client_sock)) - interrupt_sock, _ = server.accept() - stack.enter_context(closing(interrupt_sock)) - interrupt_sock.setblocking(False) + if use_signal_thread: + interrupt_sock, _ = server.accept() + stack.enter_context(closing(interrupt_sock)) + interrupt_sock.setblocking(False) + else: + interrupt_sock = None _PdbClient(pid, client_sock, interrupt_sock).cmdloop() diff --git a/Lib/test/test_remote_pdb.py b/Lib/test/test_remote_pdb.py index bef486052c9961..9c794991dd5ed9 100644 --- a/Lib/test/test_remote_pdb.py +++ b/Lib/test/test_remote_pdb.py @@ -88,6 +88,7 @@ def do_test( incoming, simulate_send_failure=False, simulate_sigint_during_stdout_write=False, + use_interrupt_socket=False, expected_outgoing=None, expected_outgoing_signals=None, expected_completions=None, @@ -175,9 +176,17 @@ def sigint_stdout_write(s): ) stack.enter_context(redirect_stdout(stdout)) - interrupt_sock = unittest.mock.Mock(spec=socket.socket) + if use_interrupt_socket: + interrupt_sock = unittest.mock.Mock(spec=socket.socket) + mock_kill = None + else: + interrupt_sock = None + mock_kill = stack.enter_context( + unittest.mock.patch("os.kill", spec=os.kill) + ) + client = _PdbClient( - pid=0, + pid=12345, server_socket=server_sock, interrupt_sock=interrupt_sock, ) @@ -204,10 +213,18 @@ def sigint_stdout_write(s): actual_state = {k: getattr(client, k) for k in expected_state} self.assertEqual(actual_state, expected_state) - outgoing_signals = [ - signal.Signals(int.from_bytes(call.args[0])) - for call in interrupt_sock.sendall.call_args_list - ] + if use_interrupt_socket: + outgoing_signals = [ + signal.Signals(int.from_bytes(call.args[0])) + for call in interrupt_sock.sendall.call_args_list + ] + else: + assert mock_kill is not None + outgoing_signals = [] + for call in mock_kill.call_args_list: + pid, signum = call.args + self.assertEqual(pid, 12345) + outgoing_signals.append(signal.Signals(signum)) self.assertEqual(outgoing_signals, expected_outgoing_signals) def test_remote_immediately_closing_the_connection(self): @@ -450,13 +467,16 @@ def test_sigint_when_writing(self): incoming = [ ("server", {"message": "Some message or other\n", "type": "info"}), ] - self.do_test( - incoming=incoming, - simulate_sigint_during_stdout_write=True, - expected_outgoing=[], - expected_outgoing_signals=[signal.SIGINT], - expected_stdout="Some message or other\n", - ) + for use_interrupt_socket in [False, True]: + with self.subTest(use_interrupt_socket=use_interrupt_socket): + self.do_test( + incoming=incoming, + simulate_sigint_during_stdout_write=True, + use_interrupt_socket=use_interrupt_socket, + expected_outgoing=[], + expected_outgoing_signals=[signal.SIGINT], + expected_stdout="Some message or other\n", + ) def test_eof_at_prompt(self): """Test signaling when a prompt gets an EOFError.""" From e1bb1d39ea8f6107c9b72a3b65455d64762ffdad Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Sun, 4 May 2025 17:48:38 -0400 Subject: [PATCH 18/18] Wrap input() calls in a function --- Lib/pdb.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 5417ba36f48de1..1173194a659dd5 100644 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -3059,11 +3059,13 @@ def _readline(self): ret, sep, self.read_buf = self.read_buf.partition(b"\n") return ret + sep - def read_command(self, prompt): - self.multiline_block = False + def read_input(self, prompt, multiline_block): + self.multiline_block = multiline_block with self._sigint_raises_keyboard_interrupt(): - reply = input(prompt) + return input(prompt) + def read_command(self, prompt): + reply = self.read_input(prompt, multiline_block=False) if self.state == "dumb": # No logic applied whatsoever, just pass the raw reply back. return reply @@ -3086,11 +3088,9 @@ def read_command(self, prompt): return prefix + reply # Otherwise, valid first line of a multi-line statement - self.multiline_block = True - continue_prompt = "...".ljust(len(prompt)) + more_prompt = "...".ljust(len(prompt)) while codeop.compile_command(reply, "", "single") is None: - with self._sigint_raises_keyboard_interrupt(): - reply += "\n" + input(continue_prompt) + reply += "\n" + self.read_input(more_prompt, multiline_block=True) return prefix + reply