From 57ec4c4f65f9bd324872aea0cfe3e04ba84e9e91 Mon Sep 17 00:00:00 2001 From: Pavel Minaev Date: Thu, 6 Aug 2020 17:43:16 -0700 Subject: [PATCH] Fix #125: Stop Debugging in a "noDebug" session doesn't kill subprocesses On Windows, run the debuggee in a separate Win32 job, and terminate the job when launcher exits. On POSIX, run the debuggee in a separate process group (PGID), and kill the entire group when launcher exits. Improve process tree autokill tests to actually check whether the child process has exited. --- src/debugpy/adapter/launchers.py | 6 +- src/debugpy/launcher/debuggee.py | 64 ++++++++++++++++++- src/debugpy/launcher/handlers.py | 1 + src/debugpy/launcher/winapi.py | 105 +++++++++++++++++++++++++++++++ tests/debugpy/test_multiproc.py | 67 ++++++++++++++++++-- 5 files changed, 233 insertions(+), 10 deletions(-) create mode 100644 src/debugpy/launcher/winapi.py diff --git a/src/debugpy/adapter/launchers.py b/src/debugpy/adapter/launchers.py index e8996167b..4d3a6e44e 100644 --- a/src/debugpy/adapter/launchers.py +++ b/src/debugpy/adapter/launchers.py @@ -172,9 +172,6 @@ def on_launcher_connected(sock): except messaging.MessageHandlingError as exc: exc.propagate(start_request) - if session.no_debug: - return - if not session.wait_for( lambda: session.launcher.pid is not None, timeout=common.PROCESS_SPAWN_TIMEOUT, @@ -183,6 +180,9 @@ def on_launcher_connected(sock): 'Timed out waiting for "process" event from launcher' ) + if session.no_debug: + return + # Wait for the first incoming connection regardless of the PID - it won't # necessarily match due to the use of stubs like py.exe or "conda run". conn = servers.wait_for_connection( diff --git a/src/debugpy/launcher/debuggee.py b/src/debugpy/launcher/debuggee.py index fc0b1c0c0..5ca0dbae6 100644 --- a/src/debugpy/launcher/debuggee.py +++ b/src/debugpy/launcher/debuggee.py @@ -5,7 +5,9 @@ from __future__ import absolute_import, division, print_function, unicode_literals import atexit +import ctypes import os +import signal import struct import subprocess import sys @@ -15,10 +17,16 @@ from debugpy.common import fmt, log, messaging, compat from debugpy.launcher import output +if sys.platform == "win32": + from debugpy.launcher import winapi + process = None """subprocess.Popen instance for the debuggee process.""" +job_handle = None +"""On Windows, the handle for the job object to which the debuggee is assigned.""" + wait_on_exit_predicates = [] """List of functions that determine whether to pause after debuggee process exits. @@ -52,6 +60,11 @@ def spawn(process_name, cmdline, env, redirect_output): else: kwargs = {} + if sys.platform != "win32": + # Start the debuggee in a new process group, so that the launcher can kill + # the entire process tree later. + kwargs.update(preexec_fn=os.setpgrp) + try: global process process = subprocess.Popen(cmdline, env=env, bufsize=0, **kwargs) @@ -61,7 +74,45 @@ def spawn(process_name, cmdline, env, redirect_output): ) log.info("Spawned {0}.", describe()) + + if sys.platform == "win32": + # Assign the debuggee to a new job object, so that the launcher can kill + # the entire process tree later. + try: + global job_handle + job_handle = winapi.kernel32.CreateJobObjectA(None, None) + + job_info = winapi.JOBOBJECT_EXTENDED_LIMIT_INFORMATION() + job_info_size = winapi.DWORD(ctypes.sizeof(job_info)) + winapi.kernel32.QueryInformationJobObject( + job_handle, + winapi.JobObjectExtendedLimitInformation, + ctypes.pointer(job_info), + job_info_size, + ctypes.pointer(job_info_size), + ) + + # Setting this flag ensures that the job will be terminated by the OS once the + # launcher exits, even if it doesn't terminate the job explicitly. + job_info.BasicLimitInformation.LimitFlags |= winapi.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + winapi.kernel32.SetInformationJobObject( + job_handle, + winapi.JobObjectExtendedLimitInformation, + ctypes.pointer(job_info), + job_info_size, + ) + + process_handle = winapi.kernel32.OpenProcess( + winapi.PROCESS_TERMINATE | winapi.PROCESS_SET_QUOTA, False, process.pid + ) + + winapi.kernel32.AssignProcessToJobObject(job_handle, process_handle) + + except Exception: + log.swallow_exception("Failed to set up job object", level="warning") + atexit.register(kill) + launcher.channel.send_event( "process", { @@ -90,16 +141,23 @@ def spawn(process_name, cmdline, env, redirect_output): try: os.close(fd) except Exception: - log.swallow_exception() + log.swallow_exception(level="warning") def kill(): if process is None: return + try: if process.poll() is None: log.info("Killing {0}", describe()) - process.kill() + # Clean up the process tree + if sys.platform == "win32": + # On Windows, kill the job object. + winapi.kernel32.TerminateJobObject(job_handle, 0) + else: + # On POSIX, kill the debuggee's process group. + os.killpg(process.pid, signal.SIGKILL) except Exception: log.swallow_exception("Failed to kill {0}", describe()) @@ -114,7 +172,7 @@ def wait_for_exit(): # taking the lowest 8 bits of that negative returncode. code &= 0xFF except Exception: - log.swallow_exception("Couldn't determine process exit code:") + log.swallow_exception("Couldn't determine process exit code") code = -1 log.info("{0} exited with code {1}", describe(), code) diff --git a/src/debugpy/launcher/handlers.py b/src/debugpy/launcher/handlers.py index 14baac493..4f5fef2ec 100644 --- a/src/debugpy/launcher/handlers.py +++ b/src/debugpy/launcher/handlers.py @@ -64,6 +64,7 @@ def property_or_debug_option(prop_name, flag_name): adapter_access_token = request("adapterAccessToken", unicode, optional=True) if adapter_access_token != (): cmdline += ["--adapter-access-token", compat.filename(adapter_access_token)] + debugpy_args = request("debugpyArgs", json.array(unicode)) cmdline += debugpy_args diff --git a/src/debugpy/launcher/winapi.py b/src/debugpy/launcher/winapi.py new file mode 100644 index 000000000..3bc6cf797 --- /dev/null +++ b/src/debugpy/launcher/winapi.py @@ -0,0 +1,105 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See LICENSE in the project root +# for license information. + +from __future__ import absolute_import, division, print_function, unicode_literals + +import ctypes +from ctypes.wintypes import BOOL, DWORD, HANDLE, LARGE_INTEGER, LPCSTR, UINT + +from debugpy.common import log + + +JOBOBJECTCLASS = ctypes.c_int +LPDWORD = ctypes.POINTER(DWORD) +LPVOID = ctypes.c_void_p +SIZE_T = ctypes.c_size_t +ULONGLONG = ctypes.c_ulonglong + + +class IO_COUNTERS(ctypes.Structure): + _fields_ = [ + ("ReadOperationCount", ULONGLONG), + ("WriteOperationCount", ULONGLONG), + ("OtherOperationCount", ULONGLONG), + ("ReadTransferCount", ULONGLONG), + ("WriteTransferCount", ULONGLONG), + ("OtherTransferCount", ULONGLONG), + ] + + +class JOBOBJECT_BASIC_LIMIT_INFORMATION(ctypes.Structure): + _fields_ = [ + ("PerProcessUserTimeLimit", LARGE_INTEGER), + ("PerJobUserTimeLimit", LARGE_INTEGER), + ("LimitFlags", DWORD), + ("MinimumWorkingSetSize", SIZE_T), + ("MaximumWorkingSetSize", SIZE_T), + ("ActiveProcessLimit", DWORD), + ("Affinity", SIZE_T), + ("PriorityClass", DWORD), + ("SchedulingClass", DWORD), + ] + + +class JOBOBJECT_EXTENDED_LIMIT_INFORMATION(ctypes.Structure): + _fields_ = [ + ("BasicLimitInformation", JOBOBJECT_BASIC_LIMIT_INFORMATION), + ("IoInfo", IO_COUNTERS), + ("ProcessMemoryLimit", SIZE_T), + ("JobMemoryLimit", SIZE_T), + ("PeakProcessMemoryUsed", SIZE_T), + ("PeakJobMemoryUsed", SIZE_T), + ] + + +JobObjectExtendedLimitInformation = JOBOBJECTCLASS(9) + +JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x00002000 + +PROCESS_TERMINATE = 0x0001 +PROCESS_SET_QUOTA = 0x0100 + + +def _errcheck(is_error_result=(lambda result: not result)): + def impl(result, func, args): + if is_error_result(result): + log.debug("{0} returned {1}", func.__name__, result) + raise ctypes.WinError() + else: + return result + + return impl + + +kernel32 = ctypes.windll.kernel32 + +kernel32.AssignProcessToJobObject.errcheck = _errcheck() +kernel32.AssignProcessToJobObject.restype = BOOL +kernel32.AssignProcessToJobObject.argtypes = (HANDLE, HANDLE) + +kernel32.CreateJobObjectA.errcheck = _errcheck(lambda result: result == 0) +kernel32.CreateJobObjectA.restype = HANDLE +kernel32.CreateJobObjectA.argtypes = (LPVOID, LPCSTR) + +kernel32.OpenProcess.errcheck = _errcheck(lambda result: result == 0) +kernel32.OpenProcess.restype = HANDLE +kernel32.OpenProcess.argtypes = (DWORD, BOOL, DWORD) + +kernel32.QueryInformationJobObject.errcheck = _errcheck() +kernel32.QueryInformationJobObject.restype = BOOL +kernel32.QueryInformationJobObject.argtypes = ( + HANDLE, + JOBOBJECTCLASS, + LPVOID, + DWORD, + LPDWORD, +) + +kernel32.SetInformationJobObject.errcheck = _errcheck() +kernel32.SetInformationJobObject.restype = BOOL +kernel32.SetInformationJobObject.argtypes = (HANDLE, JOBOBJECTCLASS, LPVOID, DWORD) + +kernel32.TerminateJobObject.errcheck = _errcheck() +kernel32.TerminateJobObject.restype = BOOL +kernel32.TerminateJobObject.argtypes = (HANDLE, UINT) diff --git a/tests/debugpy/test_multiproc.py b/tests/debugpy/test_multiproc.py index 5d13ae97a..2ee5d6bf0 100644 --- a/tests/debugpy/test_multiproc.py +++ b/tests/debugpy/test_multiproc.py @@ -4,12 +4,13 @@ from __future__ import absolute_import, division, print_function, unicode_literals +import psutil import pytest import sys import debugpy import tests -from tests import debug +from tests import debug, log from tests.debug import runners from tests.patterns import some @@ -218,18 +219,25 @@ def parent(): assert child_argv == [child, "--arg1", "--arg2", "--arg3"] -def test_autokill(pyfile, target): +@pytest.mark.parametrize("run", runners.all_launch) +def test_autokill(daemon, pyfile, target, run): @pyfile def child(): + import os + from debuggee import backchannel + + backchannel.send(os.getpid()) while True: pass @pyfile def parent(): + import debuggee import os import subprocess import sys + debuggee.setup() argv = [sys.executable, sys.argv[1]] env = os.environ.copy() subprocess.Popen( @@ -242,8 +250,9 @@ def parent(): with debug.Session() as parent_session: parent_session.expected_exit_code = some.int - - with parent_session.launch(target(parent, args=[child])): + + backchannel = parent_session.open_backchannel() + with run(parent_session, target(parent, args=[child])): pass child_config = parent_session.wait_for_next_event("debugpyAttach") @@ -253,9 +262,59 @@ def parent(): with child_session.start(): pass + child_pid = backchannel.receive() + assert child_config["subProcessId"] == child_pid + child_process = psutil.Process(child_pid) + parent_session.request("terminate") child_session.wait_for_exit() + log.info("Waiting for child process...") + child_process.wait() + + +@pytest.mark.parametrize("run", runners.all_launch) +def test_autokill_nodebug(daemon, pyfile, target, run): + @pyfile + def child(): + import os + from debuggee import backchannel + + backchannel.send(os.getpid()) + while True: + pass + + @pyfile + def parent(): + import os + import subprocess + import sys + + argv = [sys.executable, sys.argv[1]] + env = os.environ.copy() + subprocess.Popen( + argv, + env=env, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ).wait() + + with debug.Session() as session: + session.expected_exit_code = some.int + session.config["noDebug"] = True + + backchannel = session.open_backchannel() + run(session, target(parent, args=[child])) + + child_pid = backchannel.receive() + child_process = psutil.Process(child_pid) + + session.request("terminate") + + log.info("Waiting for child process...") + child_process.wait() + def test_argv_quoting(pyfile, target, run): @pyfile