Fix #503: Subprocesses are not killed when stopping the debugger

ptvsd/multiproc.py

@@ -8,8 +8,10 @@ import atexit
 import itertools
 import os
 import re
+import signal
 import socket
 import sys
+import threading
 import time
 import traceback
 
@@ -27,8 +29,15 @@ from _pydev_bundle import pydev_monkey
 from _pydevd_bundle.pydevd_comm import get_global_debugger
 
 
+subprocess_lock = threading.Lock()
+
 subprocess_listener_socket = None
 
+subprocesses = {}
+"""List of known subprocesses. Keys are process IDs, values are JsonMessageChannel
+instances; subprocess_lock must be used to synchronize access.
+"""
+
 subprocess_queue = queue.Queue()
 """A queue of incoming 'ptvsd_subprocess' notifications. Whenenever a new request
 is received, a tuple of (subprocess_request, subprocess_response) is placed in the
@@ -66,6 +75,7 @@ def listen_for_subprocesses():
 
     subprocess_listener_socket = create_server('localhost', 0)
     atexit.register(stop_listening_for_subprocesses)
+    atexit.register(kill_subprocesses)
     new_hidden_thread('SubprocessListener', _subprocess_listener).start()
 
 
@@ -80,6 +90,18 @@ def stop_listening_for_subprocesses():
     subprocess_listener_socket = None
 
 
+def kill_subprocesses():
+    with subprocess_lock:
+        pids = list(subprocesses.keys())
+    for pid in pids:
+        with subprocess_lock:
+            subprocesses.pop(pid, None)
+        try:
+            os.kill(pid, signal.SIGTERM)
+        except Exception as ex:
+            pass
+
+
 def subprocess_listener_port():
     if subprocess_listener_socket is None:
         return None
@@ -100,6 +122,8 @@ def _subprocess_listener():
 
 def _handle_subprocess(n, stream):
     class Handlers(object):
+        _pid = None
+
         def ptvsd_subprocess_request(self, request):
             # When child process is spawned, the notification it sends only
             # contains information about itself and its immediate parent.
@@ -110,12 +134,21 @@ def _handle_subprocess(n, stream):
                 'rootStartRequest': root_start_request,
             })
 
+            self._pid = arguments['processId']
+            with subprocess_lock:
+                subprocesses[self._pid] = channel
+
             debug('ptvsd_subprocess: %r' % arguments)
             response = {'incomingConnection': False}
             subprocess_queue.put((arguments, response))
             subprocess_queue.join()
             return response
 
+        def disconnect(self):
+            if self._pid is not None:
+                with subprocess_lock:
+                    subprocesses.pop(self._pid, None)
+
     name = 'SubprocessListener-%d' % n
     channel = JsonMessageChannel(stream, Handlers(), name)
     channel.start()
@@ -151,6 +184,10 @@ def notify_root(port):
         traceback.print_exc()
         sys.exit(0)
 
+    # Keep the channel open until we exit - root process uses open channels to keep
+    # track of which subprocesses are alive and which are not.
+    atexit.register(lambda: channel.close())
+
     if not response['incomingConnection']:
         debugger = get_global_debugger()
         while debugger is None:

ptvsd/wrapper.py

@@ -1143,6 +1143,8 @@ class VSCLifecycleMsgProcessor(VSCodeMessageProcessorBase):
         self._notify_ready()
 
     def on_disconnect(self, request, args):
+        multiproc.kill_subprocesses()
+
         debugger_attached.clear()
         self._restart_debugger = args.get('restart', False)
 

pytests/func/test_multiproc.py

@@ -6,11 +6,12 @@ from __future__ import print_function, with_statement, absolute_import
 
 import platform
 import pytest
+import signal
 import sys
 
 from pytests.helpers.pattern import ANY
 from pytests.helpers.session import DebugSession
-from pytests.helpers.timeline import Event, Request
+from pytests.helpers.timeline import Event, Request, Response
 
 
 @pytest.mark.timeout(60)
@@ -137,6 +138,7 @@ def test_multiprocessing(debug_session, pyfile):
 
     assert debug_session.read_json() == 'done'
 
+
 @pytest.mark.timeout(60)
 @pytest.mark.skipif(sys.version_info < (3, 0) and (platform.system() != 'Windows'),
                     reason='Bug #935')
@@ -180,10 +182,11 @@ def test_subprocess(debug_session, pyfile):
             'arguments': root_start_request.arguments,
         }
     })
+    child_pid = child_subprocess.body['processId']
     child_port = child_subprocess.body['port']
     debug_session.proceed()
 
-    child_session = DebugSession(method='attach_socket', ptvsd_port=child_port)
+    child_session = DebugSession(method='attach_socket', ptvsd_port=child_port, pid=child_pid)
     child_session.ignore_unobserved = debug_session.ignore_unobserved
     child_session.connect()
     child_session.handshake()
@@ -192,4 +195,55 @@ def test_subprocess(debug_session, pyfile):
     child_argv = debug_session.read_json()
     assert child_argv == [child, '--arg1', '--arg2', '--arg3']
 
+    child_session.wait_for_exit()
     debug_session.wait_for_exit()
+
+
+@pytest.mark.timeout(60)
+def test_subprocess_autokill(debug_session, pyfile):
+    @pyfile
+    def child():
+        while True:
+            pass
+
+    @pyfile
+    def parent():
+        import backchannel
+        import os
+        import subprocess
+        import sys
+        argv = [sys.executable]
+        argv += [sys.argv[1], '--arg1', '--arg2', '--arg3']
+        env = os.environ.copy()
+        subprocess.Popen(argv, env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        backchannel.read_json()
+
+    debug_session.multiprocess = True
+    debug_session.program_args += [child]
+    debug_session.prepare_to_run(filename=parent, backchannel=True)
+    debug_session.start_debugging()
+
+    child_subprocess = debug_session.wait_for_next(Event('ptvsd_subprocess'))
+    child_pid = child_subprocess.body['processId']
+    child_port = child_subprocess.body['port']
+
+    debug_session.proceed()
+
+    child_session = DebugSession(method='attach_socket', ptvsd_port=child_port, pid=child_pid)
+    child_session.expected_returncode = signal.SIGTERM
+    child_session.connect()
+    child_session.handshake()
+    child_session.start_debugging()
+
+    if debug_session.method == 'launch':
+        # In launch scenario, terminate the parent process by disconnecting from it.
+        debug_session.expected_returncode = signal.SIGTERM
+        disconnect = debug_session.send_request('disconnect', {})
+        debug_session.wait_for_next(Response(disconnect))
+    else:
+        # In attach scenario, just let the parent process run to completion.
+        debug_session.expected_returncode = 0
+        debug_session.write_json(None)
+
+    debug_session.wait_for_exit()
+    child_session.wait_for_exit()