From ccd9ac5f2485001bc7d074dd6b6e8a70537d60ef Mon Sep 17 00:00:00 2001 From: Fabio Zadrozny Date: Thu, 13 Feb 2020 16:16:09 -0300 Subject: [PATCH] Use launcher pid as ppid in DAP. Fixes #42 --- .../pydevd/_pydev_bundle/pydev_monkey.py | 43 +++++++++++++------ .../pydevd_command_line_handling.py | 13 ++++++ .../pydevd/_pydevd_bundle/pydevd_constants.py | 8 ++++ .../pydevd_process_net_command_json.py | 7 ++- src/debugpy/_vendored/pydevd/pydevd.py | 22 ++++++++++ .../pydevd/tests_python/debugger_unittest.py | 2 +- .../_debugger_case_pydevd_customization.py | 7 ++- .../pydevd/tests_python/test_debugger_json.py | 25 +++++++++++ .../pydevd/tests_python/test_pydev_monkey.py | 35 +++++++++------ 9 files changed, 133 insertions(+), 29 deletions(-) diff --git a/src/debugpy/_vendored/pydevd/_pydev_bundle/pydev_monkey.py b/src/debugpy/_vendored/pydevd/_pydev_bundle/pydev_monkey.py index c913075e4..3f832790a 100644 --- a/src/debugpy/_vendored/pydevd/_pydev_bundle/pydev_monkey.py +++ b/src/debugpy/_vendored/pydevd/_pydev_bundle/pydev_monkey.py @@ -3,7 +3,8 @@ import re import sys from _pydev_imps._pydev_saved_modules import threading -from _pydevd_bundle.pydevd_constants import get_global_debugger, IS_WINDOWS, IS_JYTHON, get_current_thread_id +from _pydevd_bundle.pydevd_constants import get_global_debugger, IS_WINDOWS, IS_JYTHON, get_current_thread_id, \ + sorted_dict_repr from _pydev_bundle import pydev_log from contextlib import contextmanager from _pydevd_bundle import pydevd_constants @@ -35,7 +36,7 @@ def _get_apply_arg_patching(): return getattr(_arg_patch, 'apply_arg_patching', True) -def _get_setup_updated_with_protocol(setup): +def _get_setup_updated_with_protocol_and_ppid(setup, is_exec=False): if setup is None: setup = {} setup = setup.copy() @@ -44,7 +45,11 @@ def _get_setup_updated_with_protocol(setup): setup.pop(pydevd_constants.ARGUMENT_HTTP_JSON_PROTOCOL, None) setup.pop(pydevd_constants.ARGUMENT_JSON_PROTOCOL, None) setup.pop(pydevd_constants.ARGUMENT_QUOTED_LINE_PROTOCOL, None) - setup.pop(pydevd_constants.ARGUMENT_HTTP_PROTOCOL, None) + + if not is_exec: + # i.e.: The ppid for the subprocess is the current pid. + # If it's an exec, keep it what it was. + setup[pydevd_constants.ARGUMENT_PPID] = os.getpid() protocol = pydevd_constants.get_protocol() if protocol == pydevd_constants.HTTP_JSON_PROTOCOL: @@ -65,10 +70,14 @@ def _get_setup_updated_with_protocol(setup): def _get_python_c_args(host, port, indC, args, setup): - setup = _get_setup_updated_with_protocol(setup) + setup = _get_setup_updated_with_protocol_and_ppid(setup) + + # i.e.: We want to make the repr sorted so that it works in tests. + setup_repr = setup if setup is None else (sorted_dict_repr(setup)) + return ("import sys; sys.path.insert(0, r'%s'); import pydevd; pydevd.PydevdCustomization.DEFAULT_PROTOCOL=%r; " - "pydevd.settrace(host=%r, port=%s, suspend=False, trace_only_current_thread=False, patch_multiprocessing=True, access_token=%r, client_access_token=%r); " - "from pydevd import SetupHolder; SetupHolder.setup = %s; %s" + "pydevd.settrace(host=%r, port=%s, suspend=False, trace_only_current_thread=False, patch_multiprocessing=True, access_token=%r, client_access_token=%r, __setup_holder__=%s); " + "%s" ) % ( pydev_src_dir, pydevd_constants.get_protocol(), @@ -76,7 +85,7 @@ def _get_python_c_args(host, port, indC, args, setup): port, setup.get('access-token'), setup.get('client-access-token'), - setup, + setup_repr, args[indC + 1]) @@ -227,7 +236,15 @@ def get_c_option_index(args): return ind_c -def patch_args(args): +def patch_args(args, is_exec=False): + ''' + :param list args: + Arguments to patch. + + :param bool is_exec: + If it's an exec, the current process will be replaced (this means we have + to keep the same ppid). + ''' try: pydev_log.debug("Patching args: %s", args) args = remove_quotes_from_args(args) @@ -280,7 +297,9 @@ def patch_args(args): # ['X:\\pysrc\\pydevd.py', '--multiprocess', '--print-in-debugger-startup', # '--vm_type', 'python', '--client', '127.0.0.1', '--port', '56352', '--file', 'x:\\snippet1.py'] from _pydevd_bundle.pydevd_command_line_handling import setup_to_argv - original = setup_to_argv(_get_setup_updated_with_protocol(SetupHolder.setup)) + ['--file'] + original = setup_to_argv( + _get_setup_updated_with_protocol_and_ppid(SetupHolder.setup, is_exec=is_exec) + ) + ['--file'] module_name = None m_flag = _get_str_type_compatible(args[i], '-m') @@ -459,7 +478,7 @@ def new_execl(path, *args): os.execlpe(file, arg0, arg1, ..., env) """ if _get_apply_arg_patching(): - args = patch_args(args) + args = patch_args(args, is_exec=True) send_process_created_message() return getattr(os, original_name)(path, *args) @@ -475,7 +494,7 @@ def new_execv(path, args): os.execvp(file, args) """ if _get_apply_arg_patching(): - args = patch_args(args) + args = patch_args(args, is_exec=True) send_process_created_message() return getattr(os, original_name)(path, args) @@ -491,7 +510,7 @@ def create_execve(original_name): def new_execve(path, args, env): if _get_apply_arg_patching(): - args = patch_args(args) + args = patch_args(args, is_exec=True) send_process_created_message() return getattr(os, original_name)(path, args, env) diff --git a/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_command_line_handling.py b/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_command_line_handling.py index aa48f8253..920423c77 100644 --- a/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_command_line_handling.py +++ b/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_command_line_handling.py @@ -1,3 +1,6 @@ +import os + + class ArgHandlerWithParam: ''' Handler for some arguments which needs a value @@ -48,8 +51,18 @@ def handle_argv(self, argv, i, setup): setup[self.arg_name] = True +def convert_ppid(ppid): + ret = int(ppid) + if ret != 0: + if ret == os.getpid(): + raise AssertionError( + 'ppid passed is the same as the current process pid (%s)!' % (ret,)) + return ret + + ACCEPTED_ARG_HANDLERS = [ ArgHandlerWithParam('port', int, 0), + ArgHandlerWithParam('ppid', convert_ppid, 0), ArgHandlerWithParam('vm_type'), ArgHandlerWithParam('client'), ArgHandlerWithParam('access-token'), diff --git a/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_constants.py b/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_constants.py index f34f2116a..b584ec009 100644 --- a/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_constants.py +++ b/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_constants.py @@ -347,6 +347,12 @@ def dict_iter_items(d): def dict_items(d): return d.items() + +def sorted_dict_repr(d): + s = sorted(dict_iter_items(d), key=lambda x:str(x[0])) + return '{' + ', '.join(('%r: %r' % x) for x in s) + '}' + + try: xrange = xrange except: @@ -605,6 +611,8 @@ def new_func(*args, **kwargs): HTTP_JSON_PROTOCOL = 'http_json' ARGUMENT_HTTP_JSON_PROTOCOL = 'json-dap-http' +ARGUMENT_PPID = 'ppid' + class _GlobalSettings: protocol = QUOTED_LINE_PROTOCOL diff --git a/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_process_net_command_json.py b/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_process_net_command_json.py index f2340fd63..79c0363ea 100644 --- a/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_process_net_command_json.py +++ b/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_process_net_command_json.py @@ -983,7 +983,12 @@ def on_pydevdsysteminfo_request(self, py_db, request): except AttributeError: pid = None - ppid = self.api.get_ppid() + # It's possible to have the ppid reported from args. In this case, use that instead of the + # real ppid (athough we're using `ppid`, what we want in meaning is the `launcher_pid` -- + # so, if a python process is launched from another python process, consider that process the + # parent and not any intermediary stubs). + + ppid = py_db.get_arg_ppid() or self.api.get_ppid() try: impl_desc = platform.python_implementation() diff --git a/src/debugpy/_vendored/pydevd/pydevd.py b/src/debugpy/_vendored/pydevd/pydevd.py index cb1e06618..260d22d7f 100644 --- a/src/debugpy/_vendored/pydevd/pydevd.py +++ b/src/debugpy/_vendored/pydevd/pydevd.py @@ -623,6 +623,16 @@ def new_trace_dispatch(frame, event, arg): # Stop the tracing as the last thing before the actual shutdown for a clean exit. atexit.register(stoptrace) + def get_arg_ppid(self): + try: + setup = SetupHolder.setup + if setup: + return int(setup.get('ppid', 0)) + except: + pydev_log.exception('Error getting ppid.') + + return 0 + def wait_for_ready_to_run(self): while not self.ready_to_run: # busy wait until we receive run command @@ -2487,6 +2497,9 @@ def settrace( stdout_to_server = stdout_to_server or kwargs.get('stdoutToServer', False) # Backward compatibility stderr_to_server = stderr_to_server or kwargs.get('stderrToServer', False) # Backward compatibility + # Internal use (may be used to set the setup info directly for subprocesess). + __setup_holder__ = kwargs.get('__setup_holder__') + with _set_trace_lock: _locked_settrace( host, @@ -2503,6 +2516,7 @@ def settrace( dont_trace_end_patterns, access_token, client_access_token, + __setup_holder__=__setup_holder__, ) @@ -2524,6 +2538,7 @@ def _locked_settrace( dont_trace_end_patterns, access_token, client_access_token, + __setup_holder__, ): if patch_multiprocessing: try: @@ -2541,6 +2556,8 @@ def _locked_settrace( global _global_redirect_stderr_to_server py_db = get_global_debugger() + if __setup_holder__: + SetupHolder.setup = __setup_holder__ if py_db is None: py_db = PyDB() pydevd_vm_type.setup_type() @@ -2764,6 +2781,10 @@ def settrace_forked(setup_tracing=True): setup = SetupHolder.setup if setup is None: setup = {} + else: + # i.e.: Get the ppid at this point as it just changed. + # If we later do an exec() it should remain the same ppid. + setup[pydevd_constants.ARGUMENT_PPID] = PyDevdAPI().get_ppid() access_token = setup.get('access-token') client_access_token = setup.get('client-access-token') @@ -2898,6 +2919,7 @@ def main(): # parse the command line. --file is our last argument that is required pydev_log.debug("Initial arguments: %s", (sys.argv,)) + pydev_log.debug("Current pid: %s", os.getpid()) try: from _pydevd_bundle.pydevd_command_line_handling import process_command_line setup = process_command_line(sys.argv) diff --git a/src/debugpy/_vendored/pydevd/tests_python/debugger_unittest.py b/src/debugpy/_vendored/pydevd/tests_python/debugger_unittest.py index 3e6cd5bbd..cdd6a6f2a 100644 --- a/src/debugpy/_vendored/pydevd/tests_python/debugger_unittest.py +++ b/src/debugpy/_vendored/pydevd/tests_python/debugger_unittest.py @@ -893,7 +893,7 @@ def start_socket_client(self, host, port): # 10 seconds default timeout timeout = int(os.environ.get('PYDEVD_CONNECT_TIMEOUT', 10)) s.settimeout(timeout) - for _i in range(6): + for _i in range(20): try: s.connect((host, port)) break diff --git a/src/debugpy/_vendored/pydevd/tests_python/resources/_debugger_case_pydevd_customization.py b/src/debugpy/_vendored/pydevd/tests_python/resources/_debugger_case_pydevd_customization.py index e228412cf..bac506a63 100644 --- a/src/debugpy/_vendored/pydevd/tests_python/resources/_debugger_case_pydevd_customization.py +++ b/src/debugpy/_vendored/pydevd/tests_python/resources/_debugger_case_pydevd_customization.py @@ -25,6 +25,7 @@ def main(): child_process = subprocess.Popen( [sys.executable, '-u', '-c', 'import _debugger_case_pydevd_customization;_debugger_case_pydevd_customization.call()'], stdout=subprocess.PIPE, + stderr=subprocess.PIPE, env=env, ) elif '--posix-spawn' in sys.argv: @@ -38,16 +39,20 @@ def main(): [sys.executable, '-u', '_debugger_case_pydevd_customization.py', '--simple-call'], cwd=os.path.dirname(__file__), stdout=subprocess.PIPE, + stderr=subprocess.PIPE, env=env, ) if child_process: stdout, stderr = child_process.communicate() - assert b'called' in stdout, 'Did not find b"called" in: %s' % (stdout,) + assert b'called' in stdout, 'Did not find b"called" in stdout:\n>>%s<<\nstderr:\n>>%s<<\n' % (stdout, stderr) print('TEST SUCEEDED!') # break 2 here def call(): + import pydevd + from _pydevd_bundle.pydevd_api import PyDevdAPI + assert pydevd.get_global_debugger().get_arg_ppid() == PyDevdAPI().get_ppid() print("called") # break 1 here diff --git a/src/debugpy/_vendored/pydevd/tests_python/test_debugger_json.py b/src/debugpy/_vendored/pydevd/tests_python/test_debugger_json.py index 634cfcfb0..2a92b50b5 100644 --- a/src/debugpy/_vendored/pydevd/tests_python/test_debugger_json.py +++ b/src/debugpy/_vendored/pydevd/tests_python/test_debugger_json.py @@ -2870,6 +2870,31 @@ def additional_output_checks(writer, stdout, stderr): writer.finished_ok = True +def test_ppid(case_setup, pyfile): + + @pyfile + def case_ppid(): + from pydevd import get_global_debugger + assert get_global_debugger().get_arg_ppid() == 22 + print('TEST SUCEEDED') + + def update_command_line_args(writer, args): + ret = debugger_unittest.AbstractWriterThread.update_command_line_args(writer, args) + ret.insert(ret.index('--qt-support'), '--ppid') + ret.insert(ret.index('--qt-support'), '22') + return ret + + with case_setup.test_file( + case_ppid, + update_command_line_args=update_command_line_args, + ) as writer: + json_facade = JsonFacade(writer) + json_facade.write_launch() + json_facade.write_make_initial_run() + + writer.finished_ok = True + + @pytest.mark.skipif(IS_JYTHON, reason='Flaky on Jython.') def test_path_translation_and_source_reference(case_setup): diff --git a/src/debugpy/_vendored/pydevd/tests_python/test_pydev_monkey.py b/src/debugpy/_vendored/pydevd/tests_python/test_pydev_monkey.py index 90ecbc0ae..cc4f87ed5 100644 --- a/src/debugpy/_vendored/pydevd/tests_python/test_pydev_monkey.py +++ b/src/debugpy/_vendored/pydevd/tests_python/test_pydev_monkey.py @@ -1,7 +1,12 @@ # coding: utf-8 import os import sys + import pytest + +from _pydev_bundle.pydev_monkey import pydev_src_dir +from _pydevd_bundle.pydevd_constants import sorted_dict_repr +from pydevd import SetupHolder from tests_python.debug_constants import IS_PY2 try: @@ -9,27 +14,22 @@ except: sys.path.append(os.path.dirname(os.path.dirname(__file__))) from _pydev_bundle import pydev_monkey -from pydevd import SetupHolder -from _pydev_bundle.pydev_monkey import pydev_src_dir def test_monkey(): original = SetupHolder.setup try: - SetupHolder.setup = {'client': '127.0.0.1', 'port': '0', 'protocol-quoted-line': True} + SetupHolder.setup = {'client': '127.0.0.1', 'port': '0', 'ppid': os.getpid(), 'protocol-quoted-line': True} check = '''C:\\bin\\python.exe -u -c connect(\\"127.0.0.1\\")''' debug_command = ( 'import sys; ' 'sys.path.insert(0, r\'%s\'); ' "import pydevd; pydevd.PydevdCustomization.DEFAULT_PROTOCOL='quoted-line'; " "pydevd.settrace(host='127.0.0.1', port=0, suspend=False, " - 'trace_only_current_thread=False, patch_multiprocessing=True, access_token=None, client_access_token=None); ' + 'trace_only_current_thread=False, patch_multiprocessing=True, access_token=None, client_access_token=None, __setup_holder__=%s); ' '' - "from pydevd import SetupHolder; " - "SetupHolder.setup = %s; " - '' - 'connect("127.0.0.1")') % (pydev_src_dir, SetupHolder.setup) + 'connect("127.0.0.1")') % (pydev_src_dir, sorted_dict_repr(SetupHolder.setup)) if sys.platform == "win32": debug_command = debug_command.replace('"', '\\"') debug_command = '"%s"' % debug_command @@ -55,16 +55,13 @@ def test_monkey_patch_args_indc(): original = SetupHolder.setup try: - SetupHolder.setup = {'client': '127.0.0.1', 'port': '0', 'protocol-quoted-line': True} + SetupHolder.setup = {'client': '127.0.0.1', 'port': '0', 'ppid': os.getpid(), 'protocol-quoted-line': True} check = ['C:\\bin\\python.exe', '-u', '-c', 'connect("127.0.0.1")'] debug_command = ( "import sys; sys.path.insert(0, r\'%s\'); import pydevd; pydevd.PydevdCustomization.DEFAULT_PROTOCOL='quoted-line'; " - 'pydevd.settrace(host=\'127.0.0.1\', port=0, suspend=False, trace_only_current_thread=False, patch_multiprocessing=True, access_token=None, client_access_token=None); ' - '' - "from pydevd import SetupHolder; " - "SetupHolder.setup = %s; " + 'pydevd.settrace(host=\'127.0.0.1\', port=0, suspend=False, trace_only_current_thread=False, patch_multiprocessing=True, access_token=None, client_access_token=None, __setup_holder__=%s); ' '' - 'connect("127.0.0.1")') % (pydev_src_dir, SetupHolder.setup) + 'connect("127.0.0.1")') % (pydev_src_dir, sorted_dict_repr(SetupHolder.setup)) if sys.platform == "win32": debug_command = debug_command.replace('"', '\\"') debug_command = '"%s"' % debug_command @@ -92,6 +89,8 @@ def test_monkey_patch_args_module(): '--module', '--port', '0', + '--ppid', + str(os.getpid()), '--client', '127.0.0.1', '--multiprocess', @@ -115,6 +114,8 @@ def test_monkey_patch_args_no_indc(): get_pydevd_file(), '--port', '0', + '--ppid', + str(os.getpid()), '--client', '127.0.0.1', '--protocol-quoted-line', @@ -151,6 +152,8 @@ def test_monkey_patch_args_no_indc_without_pydevd(): get_pydevd_file(), '--port', '0', + '--ppid', + str(os.getpid()), '--client', '127.0.0.1', '--protocol-quoted-line', @@ -188,6 +191,8 @@ def test_monkey_patch_c_program_arg(use_bytes): get_pydevd_file(), '--port', '0', + '--ppid', + str(os.getpid()), '--client', '127.0.0.1', '--protocol-quoted-line', @@ -213,6 +218,8 @@ def test_monkey_patch_args_module_single_arg(): '--module', '--port', '0', + '--ppid', + str(os.getpid()), '--client', '127.0.0.1', '--multiprocess',