Fix #125: Stop Debugging in a "noDebug" session doesn't kill subprocesses

int19h · int19h · commit 57ec4c4f65f9 · 2020-08-11T09:41:51.000-07:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
