From d993178d4810701c08376f9e5b48c73532bedf5e Mon Sep 17 00:00:00 2001 From: Om Singhal Date: Tue, 22 Sep 2026 14:13:44 -0400 Subject: [PATCH] Fix onTerminate being ignored on attach sessions "onTerminate" is a string option whose graceful value is "KeyboardInterrupt". launch_request validates it with str, and the debug server reads it as a string too (pydevd_process_net_command_json.py uses args.get("onTerminate", "kill")). attach_request validates it with bool. json.of_type(bool) does not reject a string, it coerces it, so "KeyboardInterrupt" arrives as True and the comparison that follows is False. _forward_terminate_request therefore stays False on every attach session, and no value of "onTerminate" can turn the option on. The visible effect is in terminate_request. On launch it delegates to the server, which interrupts the main thread, so finally blocks, atexit handlers and buffered output all get their chance. On attach it always falls through to session.finalize(terminate_debuggee=True), which kills the process and its subprocesses. The client accepts the option and then ignores it. Validate it as str, same as launch. The regression test drives the real attach handler with a parsed DAP packet and checks the flag that terminate_request reads. It stops the handler at the first argument check after "onTerminate", so it opens no sockets. --- src/debugpy/adapter/clients.py | 2 +- tests/debugpy/adapter/test_clients.py | 52 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/debugpy/adapter/clients.py b/src/debugpy/adapter/clients.py index 78a1327ed..dd8e224a9 100644 --- a/src/debugpy/adapter/clients.py +++ b/src/debugpy/adapter/clients.py @@ -447,7 +447,7 @@ def attach_request(self, request): connect = request("connect", dict, optional=True) pid = request("processId", (int, str), optional=True) sub_pid = request("subProcessId", int, optional=True) - on_terminate = request("onTerminate", bool, optional=True) + on_terminate = request("onTerminate", str, optional=True) if on_terminate: self._forward_terminate_request = on_terminate == "KeyboardInterrupt" diff --git a/tests/debugpy/adapter/test_clients.py b/tests/debugpy/adapter/test_clients.py index df1d6e13a..89ffc59b4 100644 --- a/tests/debugpy/adapter/test_clients.py +++ b/tests/debugpy/adapter/test_clients.py @@ -30,6 +30,8 @@ def write_json(self, value, encoder=None): class _FakeSession(object): """Stands in for the reentrant session lock used by the message_handler wrapper.""" + launcher = None + def __init__(self, server=None): self.server = server @@ -89,6 +91,56 @@ def test_configuration_done_out_of_order_is_rejected(start_request, has_started, assert client.has_started is has_started +def _attach(arguments): + """Runs "attach" far enough to parse its arguments, and returns the client. + + "listen" and "connect" are mutually exclusive, and that check is the first one + after "onTerminate" is read, so the handler stops before it opens any socket. + """ + stream = _MemoryStream() + channel = messaging.JsonMessageChannel(stream, None) + + client = clients.Client.__new__(clients.Client) + client.session = _FakeSession() + client.channel = channel + client.start_request = None + client.has_started = False + client._forward_terminate_request = False + client._initialize_request = messaging.Request( + channel, 1, "initialize", messaging.MessageDict(None, {}) + ) + + arguments = dict(arguments) + arguments["listen"] = {"port": 5678} + arguments["connect"] = {"port": 5678} + request = messaging.Request( + channel, 2, "attach", messaging.MessageDict(None, arguments) + ) + + with pytest.raises( + messaging.InvalidMessageError, + match='"listen" and "connect" are mutually exclusive', + ): + clients.Client.attach_request(client, request) + return client + + +@pytest.mark.parametrize( + "arguments, expected", + [ + ({"onTerminate": "KeyboardInterrupt"}, True), + ({"onTerminate": "kill"}, False), + ({}, False), + ], +) +def test_attach_honors_on_terminate(arguments, expected): + # "onTerminate" is a string option, and the debug server compares it against + # "KeyboardInterrupt". Validating it as a bool in "attach" coerced the string to + # True, so the comparison could never hold and graceful terminate was unreachable. + client = _attach(arguments) + assert client._forward_terminate_request is expected + + def test_evaluate_request_that_cannot_be_propagated_is_rejected(): stream = _MemoryStream() channel = messaging.JsonMessageChannel(stream, None)