From c0f1bf9ca80858de0db93fa5db5b337ee64cf3da Mon Sep 17 00:00:00 2001 From: Pavel Minaev Date: Wed, 7 Nov 2018 21:15:51 -0800 Subject: [PATCH] Fix #503: Subprocesses are not killed when stopping the debugger --- ptvsd/multiproc.py | 37 ++++++++++++++++++++++ ptvsd/wrapper.py | 2 ++ pytests/func/test_multiproc.py | 58 ++++++++++++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/ptvsd/multiproc.py b/ptvsd/multiproc.py index 28a0909c..547375c4 100644 --- a/ptvsd/multiproc.py +++ b/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: diff --git a/ptvsd/wrapper.py b/ptvsd/wrapper.py index 1b34d8df..31c89057 100644 --- a/ptvsd/wrapper.py +++ b/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) diff --git a/pytests/func/test_multiproc.py b/pytests/func/test_multiproc.py index c255a176..6a6754fc 100644 --- a/pytests/func/test_multiproc.py +++ b/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()