8000 gh-132975: Improve Remote PDB interrupt handling by godlygeek · Pull Request #133223 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-132975: Improve Remote PDB interrupt handling #133223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
8d89bf4
Only allow KeyboardInterrupt at specific times
godlygeek Apr 30, 2025
f65f99f
Have _PdbClient work with the socket directly
godlygeek Apr 30, 2025
ec9d3bf
Allow interrupting socket reads on Windows
godlygeek Apr 30, 2025
ed664b8
Use a SIGINT to interrupt the remote on Unix
godlygeek Apr 30, 2025
aafa48c
Handle a ValueError for operations on a closed file
godlygeek May 1, 2025
42e7cec
Use a single complex signal handler for the PDB client
godlygeek May 1, 2025
02da647
Add a news entry
godlygeek May 1, 2025
5abd63a
Update the comment to explain why we catch two exception types
godlygeek May 2, 2025
c9c5bf2
Swap signal_read/signal_write resetting to the outer finally block
godlygeek May 2, 2025
8fa88a5
Use os.kill() on every platform but Windows
godlygeek May 2, 2025
4c0b431
Use `ExitStack` to reduce nesting in `attach`
godlygeek May 2, 2025
5993e06
Use a thread to manage interrupts on Windows
godlygeek May 2, 2025
dd7c9b1
U 8000 se the signal handling thread approach on all platforms
godlygeek May 4, 2025
e2391b0
Make all _connect arguments keyword-only
godlygeek May 4, 2025
edd7517
Merge remote-tracking branch 'upstream/main' into improve_remote_pdb_…
godlygeek May 4, 2025
14aa8e7
One line for contextlib imports
godlygeek May 4, 2025
b26ed5c
Add some tests for handling SIGINT in the PDB client
godlygeek May 4, 2025
a860087
Switch back to os.kill() on Unix
godlygeek May 4, 2025
e1bb1d3
Wrap input() calls in a function
godlygeek May 4, 2025
2a7807c
Merge branch 'main' into improve_remote_pdb_interrupt_handling
pablogsal May 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use the signal handling thread approach on all platforms
Let the caller of `_connect` tell us whether to start the thread.
  • Loading branch information
godlygeek committed May 4, 2025
commit dd7c9b1d877f2c8eaf6dced33b0d7757034c4a2a
125 changes: 64 additions & 61 deletions Lib/pdb.py
8000
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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)
)
Expand All @@ -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,
)
"""
)
Expand All @@ -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()

Expand Down
26 chang 8000 es: 8 additions & 18 deletions Lib/test/test_remote_pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -1105,6 +1089,7 @@ def bar():
frame=frame,
commands="",
version=pdb._PdbServer.protocol_version(),
signal_raising_thread=True,
)
print("Connected to debugger")
iterations = 50
Expand All @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
0