From 0ea50467aaa0574bc043d407229411170de4c1b3 Mon Sep 17 00:00:00 2001 From: Pavel Minaev Date: Wed, 31 Jul 2019 13:05:06 -0700 Subject: [PATCH] Fix #1488: Handling launch (spawn ptvsd) (#1647) Fix #1605: Debuggee process lifetime management Mitigate #1637: log.exception() doesn't capture the full stack Handle "launch" request, parse and validate the debug configuration, and spawn the debuggee process with debug server. Track debuggee process and its subprocesses, and kill them as needed. Refactor Singleton and ThreadSafeSingleton to allow for easier synchronization between IDE and server message handlers without excessive locking. Fix various corner cases and race conditions in disconnect scenarios. Make log.exception() log the calling stack, not just the exception stack. Add JSON property validation to MessageDict. Add --log-stderr switch to the server to enable full logging to stderr. Add --cls switch to the adapter to reset terminal before logging anything (for convenience when debugging it). Add some printf-debugging helpers. --- .flake8 | 2 +- .vscode/launch.json | 9 +- ptvsd.code-workspace | 2 + src/ptvsd/adapter/__init__.py | 6 +- src/ptvsd/adapter/__main__.py | 17 +- src/ptvsd/adapter/channels.py | 35 +-- src/ptvsd/adapter/contract.py | 59 +---- src/ptvsd/adapter/debuggee.py | 397 +++++++++++++++++++++++++++++++++- src/ptvsd/adapter/messages.py | 266 +++++++++++++++-------- src/ptvsd/adapter/state.py | 19 +- src/ptvsd/common/json.py | 187 ++++++++++++++++ src/ptvsd/common/launcher.py | 5 + src/ptvsd/common/log.py | 30 ++- src/ptvsd/common/messaging.py | 48 ++++ src/ptvsd/common/singleton.py | 121 +++++++++-- src/ptvsd/server/__main__.py | 10 + 16 files changed, 1013 insertions(+), 200 deletions(-) diff --git a/.flake8 b/.flake8 index 8adba1ed..4b32f215 100644 --- a/.flake8 +++ b/.flake8 @@ -1,5 +1,5 @@ [flake8] -ignore = W, +ignore = W,BLK, E24,E121,E123,E125,E126,E221,E226,E266,E704, E265,E722,E501,E731,E306,E401,E302,E222,E303, E402,E305,E261,E262,E203 diff --git a/.vscode/launch.json b/.vscode/launch.json index 135628a2..3c7b5132 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -9,8 +9,9 @@ "type": "python", "request": "launch", "console": "integratedTerminal", + "consoleTitle": "ptvsd.adapter", "program": "${workspaceFolder}/src/ptvsd/adapter", - "args": ["--debug-server", "8765"], + "args": ["--debug-server", "8765", "--cls"], "customDebugger": true, }, @@ -20,9 +21,13 @@ "type": "python", "request": "launch", "debugServer": 8765, - "console": "integratedTerminal", + "console": "internalConsole", + //"console": "integratedTerminal", + //"console": "externalTerminal", + "consoleTitle": "ptvsd.server", //"program": "${file}", "program": "${workspaceFolder}/tests/test_data/testpkgs/pkg1/__main__.py", + //"ptvsdArgs": ["--log-stderr"], }, { "name": "Attach [debugServer]", diff --git a/ptvsd.code-workspace b/ptvsd.code-workspace index 1b616d80..8fe2ebf9 100644 --- a/ptvsd.code-workspace +++ b/ptvsd.code-workspace @@ -8,6 +8,8 @@ "python.linting.enabled": true, "python.linting.pylintEnabled": false, "python.envFile": "${workspaceFolder}/.env", + "python.formatting.provider": "black", + "files.exclude": { "**/*.pyc": true, "**/__pycache__": true, diff --git a/src/ptvsd/adapter/__init__.py b/src/ptvsd/adapter/__init__.py index d1a64c66..e055b585 100644 --- a/src/ptvsd/adapter/__init__.py +++ b/src/ptvsd/adapter/__init__.py @@ -4,5 +4,9 @@ from __future__ import absolute_import, print_function, unicode_literals - __all__ = [] + +import os.path + +# Force absolute path on Python 2. +__file__ = os.path.abspath(__file__) diff --git a/src/ptvsd/adapter/__main__.py b/src/ptvsd/adapter/__main__.py index e14da1f6..9d706d4b 100644 --- a/src/ptvsd/adapter/__main__.py +++ b/src/ptvsd/adapter/__main__.py @@ -16,14 +16,19 @@ def main(args): from ptvsd.common import log from ptvsd.adapter import channels + if args.cls: + print("\033c") + + log.stderr_levels |= {"info"} + log.filename_prefix = "ptvsd.adapter" log.to_file() if args.debug_server is None: address = None else: address = ("localhost", args.debug_server) - # If in debugServer mode, log everything to stderr. - log.stderr_levels = set(log.LEVELS) + # If in debugServer mode, log "debug" to stderr as well. + log.stderr_levels |= {"debug"} chan = channels.Channels() ide = chan.connect_to_ide(address) @@ -52,6 +57,7 @@ def main(args): def _parse_argv(): parser = argparse.ArgumentParser() + parser.add_argument( "-d", "--debug-server", @@ -62,6 +68,13 @@ def _parse_argv(): metavar="PORT", help="start the adapter in debugServer mode on the specified port", ) + + parser.add_argument( + "--cls", + action='store_true', + help="clear screen before starting the debuggee", + ) + return parser.parse_args() diff --git a/src/ptvsd/adapter/channels.py b/src/ptvsd/adapter/channels.py index 5c067a09..36dd5c1c 100644 --- a/src/ptvsd/adapter/channels.py +++ b/src/ptvsd/adapter/channels.py @@ -53,14 +53,14 @@ class Channels(singleton.ThreadSafeSingleton): ide_stream = messaging.JsonIOStream.from_stdio("IDE") else: host, port = address - server_sock = socket.create_server(host, port) + listener = socket.create_server(host, port) try: log.info( - "ptvsd debugServer waiting for connection on {0}:{1}...", host, port + "Adapter waiting for connection from IDE on {0}:{1}...", host, port ) - sock, (ide_host, ide_port) = server_sock.accept() + sock, (ide_host, ide_port) = listener.accept() finally: - server_sock.close() + listener.close() log.info("IDE connection accepted from {0}:{1}.", ide_host, ide_port) ide_stream = messaging.JsonIOStream.from_socket(sock, "IDE") @@ -96,12 +96,16 @@ class Channels(singleton.ThreadSafeSingleton): return self._server @singleton.autolocked_method - def accept_connection_from_server(self, address): + def accept_connection_from_server(self, address, before_accept=(lambda _: None)): """Creates a DAP message channel to the server, and returns that channel. The channel is established by listening on the specified address until there is an incoming TCP connection. Only one incoming connection is accepted. + before_accept((host, port)) is invoked after the listener socket has been + set up, but before the thread blocks waiting for incoming connection. This + provides access to the actual port number if port=0. + Caller is responsible for calling start() on the returned channel. """ @@ -111,21 +115,24 @@ class Channels(singleton.ThreadSafeSingleton): from ptvsd.adapter import messages host, port = address - server_sock = socket.create_server(host, port) + listener = socket.create_server(host, port) + host, port = listener.getsockname() + log.info("Adapter waiting for connection from debug server on {0}:{1}...", host, port) + before_accept((host, port)) + try: - log.info( - "ptvsd adapter waiting for connection on {0}:{1}...", host, port - ) - sock, (server_host, server_port) = server_sock.accept() + sock, (server_host, server_port) = listener.accept() finally: - server_sock.close() - log.info("Debug server connection accepted from {0}:{1}.", server_host, server_port) + listener.close() + log.info( + "Debug server connection accepted from {0}:{1}.", server_host, server_port + ) server_stream = messaging.JsonIOStream.from_socket(sock, "server") - self._server = messaging.JsonMessageChannel( + self._server = server = messaging.JsonMessageChannel( server_stream, messages.ServerMessages(), server_stream.name ) - return self._server + return server @singleton.autolocked_method def close_server(self): diff --git a/src/ptvsd/adapter/contract.py b/src/ptvsd/adapter/contract.py index 4e126863..ab262fa2 100644 --- a/src/ptvsd/adapter/contract.py +++ b/src/ptvsd/adapter/contract.py @@ -7,7 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals """Runtime contracts for the IDE and the server. """ -from ptvsd.common import fmt, log, singleton +from ptvsd.common import fmt, json, log, singleton class Capabilities(dict): @@ -20,13 +20,9 @@ class Capabilities(dict): by instances of this class. Keys are names, and values are either default values or validators. - If the value is callable, it is a validator. The validator is invoked with the - actual value of the JSON property passed to it as the sole argument; or if the - property is missing in JSON, then () is passed. The validator must either raise - ValueError describing why the property value is invalid, or return the value; - in case where () was passed, it must return the default value replacing that. - - If the value is not callable, it is as if default(value) validator was used. + If the value is callable, it must be a JSON validator; see ptvsd.common.json for + details. If the value is not callable, it is as if json.default(value) validator + was used instead. """ def __init__(self, message): @@ -40,61 +36,24 @@ class Capabilities(dict): for name, validate in self.PROPERTIES.items(): value = payload.get(name, ()) if not callable(validate): - validate = default(validate) + validate = json.default(validate) try: value = validate(value) except Exception as exc: - message.isnt_valid("{0!r} {1}", name, exc) + message.isnt_valid("{0!j} {1}", name, exc) assert value != (), fmt( - "{0!r} must provide a default value for missing properties.", validate + "{0!j} must provide a default value for missing properties.", validate ) self[name] = value - log.info("{0}", self) + log.debug("{0}", self) def __repr__(self): return fmt("{0}: {1!j}", type(self).__name__, dict(self)) -def default(default): - """Returns a validator for a JSON property with a default value. - - The validator will only allow property values that have the same type as the - specified default value. - """ - - def validate(value): - if value == (): - return default - elif isinstance(value, type(default)): - return value - else: - raise ValueError(fmt("must be a {0}", type(default).__name__)) - - return validate - - -def enum(*values): - """Returns a validator for a JSON enum. - - The validator will only allow property values that match one of those specified. - If property is missing, the first value specified is used as the default. - """ - - def validate(value): - if value == (): - return values[0] - elif value in values: - return value - else: - raise ValueError(fmt("must be one of: {0!r}", list(values))) - - assert len(values) - return validate - - class IDECapabilities(Capabilities): PROPERTIES = { "supportsVariableType": False, @@ -143,7 +102,7 @@ class IDEExpectations(Capabilities): "locale": "en-US", "linesStartAt1": True, "columnsStartAt1": True, - "pathFormat": enum("path", "uri"), + "pathFormat": json.enum("path"), # we don't support "uri" } diff --git a/src/ptvsd/adapter/debuggee.py b/src/ptvsd/adapter/debuggee.py index 4742cf30..8c21451c 100644 --- a/src/ptvsd/adapter/debuggee.py +++ b/src/ptvsd/adapter/debuggee.py @@ -4,7 +4,23 @@ from __future__ import absolute_import, print_function, unicode_literals +"""Manages the lifetime of the debugged process and its subprocesses, in scenarios +where it is controlled by the adapter (i.e. "launch"). +""" + import atexit +import collections +import os +import platform +import signal +import subprocess +import sys +import threading + +import ptvsd.__main__ +from ptvsd.adapter import channels, contract +from ptvsd.common import compat, fmt, json, launcher, messaging, log, singleton +from ptvsd.common.compat import unicode terminate_at_exit = True @@ -12,25 +28,386 @@ terminate_at_exit = True or allowed to continue running. """ +exit_code = None +"""The exit code of the debuggee process, once it has terminated.""" -def launch_and_connect(request): - """Launch the process as requested by the DAP "launch" request, with the debug - server running inside the process; and connect to that server. +pid = None +"""Debuggee process ID.""" + +_exited = None +"""A threading.Event that is set when the debuggee process exits. + +Created when the process is spawned. +""" + + +SpawnInfo = collections.namedtuple( + "SpawnInfo", ["console", "console_title", "cmdline", "cwd", "env"] +) + + +def spawn_and_connect(request): + """Spawns the process as requested by the DAP "launch" request, with the debug + server running inside the process; and connects to that server. Returns the + server channel. + + Caller is responsible for calling start() on the returned channel. """ - raise NotImplementedError + channel = channels.Channels().accept_connection_from_server( + ("127.0.0.1", 0), + before_accept=lambda address: _parse_request_and_spawn(request, address), + ) + return channel + + +def _parse_request_and_spawn(request, address): + spawn_info = _parse_request(request, address) + log.debug( + "SpawnInfo = {0!j}", + collections.OrderedDict( + { + "console": spawn_info.console, + "cwd": spawn_info.cwd, + "cmdline": spawn_info.cmdline, + "env": spawn_info.env, + } + ), + ) + + spawn = { + "internalConsole": _spawn_popen, + "integratedTerminal": _spawn_terminal, + "externalTerminal": _spawn_terminal, + }[spawn_info.console] + + global _exited + _exited = threading.Event() + try: + spawn(request, spawn_info) + finally: + if pid is None: + _exited.set() + else: + atexit.register(lambda: terminate() if terminate_at_exit else None) + + +def _parse_request(request, address): + """Parses a "launch" request and returns SpawnInfo for it. + + address is (host, port) on which the adapter listener is waiting for connection + from the debug server. + """ + + assert request.is_request("launch") + host, port = address + debug_options = set(request("debugOptions", json.array(unicode))) + + # Handling of properties that can also be specified as legacy "debugOptions" flags. + # If property is explicitly set to false, but the flag is in "debugOptions", treat + # it as an error. + def property_or_debug_option(prop_name, flag_name): + assert prop_name[0].islower() and flag_name[0].isupper() + value = request(prop_name, json.default(flag_name in debug_options)) + if value is False and flag_name in debug_options: + request.isnt_valid( + '{0!r}:false and "debugOptions":[{1!r}] are mutually exclusive', + prop_name, + flag_name, + ) + return value + + console = request( + "console", + json.enum( + "internalConsole", "integratedTerminal", "externalTerminal", optional=True + ), + ) + if console != "internalConsole": + if not contract.ide.capabilities["supportsRunInTerminalRequest"]: + request.cant_handle( + 'Unable to launch via "console":{0!j}, because the IDE is does not ' + 'have the "supportsRunInTerminalRequest" capability', + console, + ) + + console_title = request("consoleTitle", json.default("Python Debug Console")) + + cmdline = [] + if property_or_debug_option("sudo", "Sudo"): + if platform.system() == "Windows": + request.cant_handle('"sudo":true is not supported on Windows.') + else: + cmdline += ["sudo"] + + # "pythonPath" is a deprecated legacy spelling. If "python" is missing, then try + # the alternative. But if both are missing, the error message should say "python". + python_key = "python" + if python_key in request: + if "pythonPath" in request: + request.isnt_valid('"pythonPath" is not valid if "python" is specified') + elif "pythonPath" in request: + python_key = "pythonPath" + python = request(python_key, json.array(unicode, vectorize=True, size=(1,))) + if not len(python): + python = [sys.executable] + cmdline += python + + cmdline += [compat.filename(launcher.__file__)] + if property_or_debug_option("waitOnNormalExit", "WaitOnNormalExit"): + cmdline += ["--wait-on-normal"] + if property_or_debug_option("waitOnAbnormalExit", "WaitOnAbnormalExit"): + cmdline += ["--wait-on-abnormal"] + + ptvsd_args = request("ptvsdArgs", json.array(unicode)) + cmdline += [ + "--", + compat.filename(ptvsd.__main__.__file__), + "--client", + "--host", + host, + "--port", + str(port), + ] + ptvsd_args + + program = module = code = () + if "program" in request: + program = request("program", json.array(unicode, vectorize=True, size=(1,))) + cmdline += program + if "module" in request: + module = request("module", json.array(unicode, vectorize=True, size=(1,))) + cmdline += ["-m"] + cmdline += module + if "code" in request: + code = request("code", json.array(unicode, vectorize=True, size=(1,))) + cmdline += ["-c"] + cmdline += code + + num_targets = len([x for x in (program, module, code) if x != ()]) + if num_targets == 0: + request.isnt_valid('either "program", "module", or "code" must be specified') + elif num_targets != 1: + request.isnt_valid('"program", "module", and "code" are mutually exclusive') + + cmdline += request("args", json.array(unicode)) + + cwd = request("cwd", unicode, optional=True) + if cwd == (): + # If it's not specified, but we're launching a file rather than a module, + # and the specified path has a directory in it, use that. + cwd = None if program == () else (os.path.dirname(program) or None) + + env = request("env", json.object(unicode)) + + return SpawnInfo(console, console_title, cmdline, cwd, env) + + +def _spawn_popen(request, spawn_info): + env = os.environ.copy() + env.update(spawn_info.env) + + try: + proc = subprocess.Popen(spawn_info.cmdline, cwd=spawn_info.cwd, env=env) + except Exception as exc: + request.cant_handle( + "Error launching process: {0}\n\nCommand line:{1!r}", + exc, + spawn_info.cmdline, + ) + + log.info("Spawned debuggee process with PID={0}.", proc.pid) + + global pid + try: + pid = proc.pid + ProcessTracker().track(pid) + except Exception: + # If we can't track it, we won't be able to terminate it if asked; but aside + # from that, it does not prevent debugging. + log.exception( + "Unable to track debuggee process with PID={0}.", pid, category="warning" + ) + + # Wait directly on the Popen object, instead of going via ProcessTracker. This is + # more reliable on Windows, because Popen always has the correct process handle + # that it gets from CreateProcess, whereas ProcessTracker will use OpenProcess to + # get it from PID, and there's a race condition there if the process dies and its + # PID is reused before OpenProcess is called. + def wait_for_exit(): + global exit_code + try: + exit_code = proc.wait() + except Exception: + log.exception("Couldn't determine process exit code:") + exit_code = -1 + finally: + _exited.set() + + wait_thread = threading.Thread(target=wait_for_exit, name='"launch" worker') + wait_thread.start() + + +def _spawn_terminal(request, spawn_info): + kinds = {"integratedTerminal": "integrated", "externalTerminal": "external"} + body = { + "kind": kinds[spawn_info.console], + "title": spawn_info.console_title, + "cwd": spawn_info.cwd, + "args": spawn_info.cmdline, + "env": spawn_info.env, + } + + try: + result = channels.Channels().ide().request("runInTerminal", body) + except messaging.MessageHandlingError as exc: + exc.propagate(request) + + global pid + pid = result("processId", int) + + try: + ProcessTracker().track(pid, after_exit=lambda: _exited.set()) + except Exception as exc: + # If we can't track it, we won't be able to detect if it exited or retrieve + # the exit code, so fail immediately. + request.cant_handle("Couldn't get debuggee process handle: {0}", str(exc)) + + +def wait_for_exit(timeout=None): + """Waits for the debugee process to exit. + + Returns True if the process exited, False if the wait timed out. If it returned + True, then exit_code is guaranteed to be set. + """ + + assert _exited is not None + timed_out = not _exited.wait(timeout) + if not timed_out: + # ProcessTracker will stop tracking it by itself, but it might take a bit + # longer for it to notice that the process is gone. If killall() is invoked + # before that, it will try to kill that non-existing process, and log the + # resulting error. This prevents that. + ProcessTracker().stop_tracking(pid) + return not timed_out def terminate(after=0): - """Terminate the debuggee process, if it is still alive after the specified time. + """Waits for the debugee process to exit for the specified number of seconds. If + the process or any subprocesses are still alive after that time, force-kills them. + + If any errors occur while trying to kill any process, logs and swallows them. + + If the debugee process hasn't been spawned yet, does nothing. """ - pass # TODO + if _exited is None: + return + + wait_for_exit(after) + ProcessTracker().killall() -def _atexit_handler(): - if terminate_at_exit: - terminate() +def register_subprocess(pid): + """Registers a subprocess of the debuggee process.""" + ProcessTracker().track(pid) -atexit.register(_atexit_handler) +class ProcessTracker(singleton.ThreadSafeSingleton): + """Tracks processes that belong to the debuggee. + """ + + _processes = {} + """Keys are PIDs, and values are handles as used by os.waitpid(). On Windows, + handles are distinct. On all other platforms, the PID is also the handle. + """ + + _exit_codes = {} + """Keys are PIDs, values are exit codes.""" + + @singleton.autolocked_method + def track(self, pid, after_exit=lambda _: None): + """Starts tracking the process with the specified PID, and returns its handle. + + If the process exits while it is still being tracked, after_exit is invoked + with its exit code. + """ + + # Register the atexit handler only once, on the first tracked process. + if not len(self._processes): + atexit.register(lambda: self.killall() if terminate_at_exit else None) + + self._processes[pid] = handle = _pid_to_handle(pid) + log.debug( + "Tracking debuggee process with PID={0} and HANDLE=0x{1:08X}.", pid, handle + ) + + def wait_for_exit(): + try: + _, exit_code = os.waitpid(handle, 0) + except Exception: + exit_code = -1 + log.exception( + "os.waitpid() for debuggee process with HANDLE=0x{1:08X} failed:", + handle, + ) + else: + exit_code >>= 8 + log.info( + "Debuggee process with PID={0} exited with exitcode {1}.", + pid, + exit_code, + ) + + with self: + if pid in self._processes: + self._exit_codes[pid] = exit_code + self.stop_tracking(pid) + after_exit(exit_code) + + wait_thread = threading.Thread( + target=wait_for_exit, name=fmt("Process(pid={0}) tracker", pid) + ) + wait_thread.daemon = True + wait_thread.start() + + return handle + + @singleton.autolocked_method + def stop_tracking(self, pid): + if self._processes.pop(pid, None) is not None: + log.debug("Stopped tracking debuggee process with PID={0}.", pid) + + @singleton.autolocked_method + def killall(self): + pids = list(self._processes.keys()) + for pid in pids: + log.info("Killing debuggee process with PID={0}.", pid) + try: + os.kill(pid, signal.SIGTERM) + except Exception: + log.exception("Couldn't kill debuggee process with PID={0}:", pid) + + +if platform.system() != "Windows": + _pid_to_handle = lambda pid: pid +else: + import ctypes + from ctypes import wintypes + + class ProcessAccess(wintypes.DWORD): + PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 + SYNCHRONIZE = 0x100000 + + OpenProcess = ctypes.windll.kernel32.OpenProcess + OpenProcess.restype = wintypes.HANDLE + OpenProcess.argtypes = (ProcessAccess, wintypes.BOOL, wintypes.DWORD) + + def _pid_to_handle(pid): + handle = OpenProcess( + ProcessAccess.PROCESS_QUERY_LIMITED_INFORMATION | ProcessAccess.SYNCHRONIZE, + False, + pid, + ) + if not handle: + raise ctypes.WinError() + return handle diff --git a/src/ptvsd/adapter/messages.py b/src/ptvsd/adapter/messages.py index 9a1e186b..447e320d 100644 --- a/src/ptvsd/adapter/messages.py +++ b/src/ptvsd/adapter/messages.py @@ -7,27 +7,35 @@ from __future__ import absolute_import, print_function, unicode_literals import functools import ptvsd -from ptvsd.common import log, messaging, singleton -from ptvsd.adapter import channels, contract, debuggee, options, state +from ptvsd.common import json, log, messaging, singleton +from ptvsd.adapter import channels, debuggee, contract, options, state -class Shared(singleton.ThreadSafeSingleton): +class _Shared(singleton.ThreadSafeSingleton): """Global state shared between IDE and server handlers, other than contracts. """ + # Only attributes that are set by IDEMessages and marked as readonly before + # connecting to the server can go in here. + threadsafe_attrs = {"start_method", "terminate_on_disconnect"} + + start_method = None + """Either "launch" or "attach", depending on the request used.""" + + terminate_on_disconnect = True + """Whether the debuggee process should be terminated on disconnect.""" + class Messages(singleton.Singleton): # Misc helpers that are identical for both IDEMessages and ServerMessages. - _channels = channels.Channels() - # Shortcut for the IDE channel. This one does not check for None, because in the # normal stdio channel scenario, the channel will never disconnect. The debugServer # scenario is for testing purposes only, so it's okay to crash if IDE suddenly # disconnects in that case. @property def _ide(self): - return self._channels.ide() + return _channels.ide() @property def _server(self): @@ -36,7 +44,7 @@ class Messages(singleton.Singleton): To test whether it is available or not, use _channels.server() instead, following the guidelines in its docstring. """ - server = self._channels.server() + server = _channels.server() if server is None: messaging.Message.isnt_valid( "Connection to debug server is not established yet" @@ -97,13 +105,17 @@ class IDEMessages(Messages): ], } + # Until the server message loop is, this isn't really shared, so we can simplify + # synchronization by keeping it exclusive until then. This way, all attributes + # that are computed during initialization and never change after don't need to be + # synchronized at all. + _shared = _Shared(shared=False) + # Until "launch" or "attach", there's no debug server yet, and so we can't propagate # messages. But they will need to be replayed once we establish connection to server, # so store them here until then. After all messages are replayed, it is set to None. _initial_messages = [] - terminate_on_disconnect = True - # A decorator to add the message to initial_messages if needed before handling it. # Must be applied to the handler for every message that can be received before # connection to the debug server can be established while handling attach/launch, @@ -122,7 +134,7 @@ class IDEMessages(Messages): # case some events appear in future protocol versions. @_replay_to_server def event(self, event): - server = self._channels.server() + server = _channels.server() if server is not None: server.propagate(event) @@ -140,9 +152,14 @@ class IDEMessages(Messages): # Handles various attributes common to both "launch" and "attach". def _debug_config(self, request): assert request.command in ("launch", "attach") + self._shared.start_method = request.command + _Shared.readonly_attrs.add("start_method") - # TODO: Change all old VS style debugger settings to debugOptions - # See https://github.com/microsoft/ptvsd/issues/1219 + # We're about to connect to the server and start the message loop for its + # handlers, so _shared is actually going to be shared from now on. + self._shared.share() + + # TODO: handle "logToFile". Maybe also "trace" (to Debug Output) like Node.js? pass @_replay_to_server @@ -151,19 +168,20 @@ class IDEMessages(Messages): self._debug_config(request) # TODO: nodebug - debuggee.launch_and_connect(request) + debuggee.spawn_and_connect(request) return self._configure() @_replay_to_server @_only_allowed_while("initializing") def attach_request(self, request): - self.terminate_on_disconnect = False + self._shared.terminate_on_disconnect = False + _Shared.readonly_attrs.add("terminate_on_disconnect") self._debug_config(request) options.host = request.arguments.get("host", options.host) options.port = int(request.arguments.get("port", options.port)) - self._channels.connect_to_server(address=(options.host, options.port)) + _channels.connect_to_server(address=(options.host, options.port)) return self._configure() @@ -208,86 +226,56 @@ class IDEMessages(Messages): ServerMessages().release_events() return ret - # Common part of the handlers for "disconnect" and "terminate" requests. - def _shutdown(self, request, terminate): - if request.arguments.get("restart", False): + def _disconnect_or_terminate_request(self, request): + assert request.is_request("disconnect") or request.is_request("terminate") + + if request("restart", json.default(False)): request.isnt_valid("Restart is not supported") - # "disconnect" or "terminate" can come before we even connect to the server. - server = self._channels.server() - server_exc = None - result = {} - if server is not None: - try: - result = server.delegate(request) - except messaging.MessageHandlingError as exc: - # If the server was there, but failed to handle the request, we want - # to propagate that failure back to the IDE - but only after we have - # recorded the state transition and terminated the debuggee if needed. - server_exc = exc - except Exception: - # The server might have already disconnected - this is not an error. - pass + terminate = (request.command == "terminate") or request( + "terminateDebuggee", json.default(self._shared.terminate_on_disconnect) + ) - state.change("shutting_down") + server = _channels.server() + server_exc = None + terminate_requested = False + result = {} + + try: + state.change("shutting_down") + except state.InvalidStateTransition: + # Can happen if the IDE or the server disconnect while we were handling + # this. If it was the server, we want to move on so that we can report + # to the IDE before exiting. If it was the IDE, disconnect() handler has + # already dealt with the server, and there isn't anything else we can do. + pass + else: + if server is not None: + try: + result = server.delegate(request) + except messaging.MessageHandlingError as exc: + # If the server was there, but failed to handle the request, we want + # to propagate that failure back to the IDE - but only after we have + # recorded the state transition and terminated the debuggee if needed. + server_exc = exc + except Exception: + # The server might have already disconnected - this is not an error. + pass + else: + terminate_requested = terminate if terminate: - debuggee.terminate() + # If we asked the server to terminate, give it some time to do so before + # we kill the debuggee process. Otherwise, just kill it immediately. + debuggee.terminate(5 if terminate_requested else 0) if server_exc is None: return result else: server_exc.propagate(request) - @_only_allowed_while("initializing", "configuring", "running") - def disconnect_request(self, request): - # We've already decided earlier based on whether it was "launch" or "attach", - # but let the request override that. - terminate = request.arguments.get( - "terminateDebuggee", self.terminate_on_disconnect - ) - return self._shutdown(request, terminate) - - @_only_allowed_while("initializing", "configuring", "running") - def terminate_request(self, request): - return self._shutdown(request, terminate=True) - - # Adapter's stdout was closed by IDE. - def disconnect(self): - try: - if state.current() == "shutting_down": - # Graceful disconnect. We have already received "disconnect" or - # "terminate", and delegated it to the server. Nothing to do. - return - - # Can happen if the IDE was force-closed or crashed. - log.warning('IDE disconnected without sending "disconnect" or "terminate".') - state.change("shutting_down") - - server = self._channels.server() - if server is None: - if self.terminate_on_disconnect: - # It happened before we connected to the server, so we cannot gracefully - # terminate the debuggee. Force-kill it immediately. - debuggee.terminate() - return - - # Try to shut down the server gracefully, even though the adapter wasn't. - command = "terminate" if self.terminate_on_disconnect else "disconnect" - try: - server.send_request(command) - except Exception: - # The server might have already disconnected as well, or it might fail - # to handle the request. But we can't report failure to the IDE at this - # point, and it's already logged, so just move on. - pass - - finally: - if self.terminate_on_disconnect: - # If debuggee is still there, give it some time to terminate itself, - # then force-kill. Since the IDE is gone already, and nobody is waiting - # for us to respond, there's no rush. - debuggee.terminate(after=60) + disconnect_request = _disconnect_or_terminate_request + terminate_request = _disconnect_or_terminate_request @_only_allowed_while("running") def pause_request(self, request): @@ -304,7 +292,7 @@ class IDEMessages(Messages): @_only_allowed_while("configuring", "running") def ptvsd_systemInfo_request(self, request): result = {"ptvsd": {"version": ptvsd.__version__}} - server = self._channels.server() + server = _channels.server() if server is not None: try: pydevd_info = server.request("pydevdSystemInfo") @@ -316,23 +304,67 @@ class IDEMessages(Messages): result.update(pydevd_info) return result + # Adapter's stdout was closed by IDE. + def disconnect(self): + terminate_on_disconnect = self._shared.terminate_on_disconnect + try: + try: + state.change("shutting_down") + except state.InvalidStateTransition: + # Either we have already received "disconnect" or "terminate" from the + # IDE and delegated it to the server, or the server dropped connection. + # Either way, everything that needed to be done is already done. + return + else: + # Can happen if the IDE was force-closed or crashed. + log.warning( + 'IDE disconnected without sending "disconnect" or "terminate".' + ) + + server = _channels.server() + if server is None: + if terminate_on_disconnect: + # It happened before we connected to the server, so we cannot gracefully + # terminate the debuggee. Force-kill it immediately. + debuggee.terminate() + return + + # Try to shut down the server gracefully, even though the adapter wasn't. + command = "terminate" if terminate_on_disconnect else "disconnect" + try: + server.send_request(command) + except Exception: + # The server might have already disconnected as well, or it might fail + # to handle the request. But we can't report failure to the IDE at this + # point, and it's already logged, so just move on. + pass + + finally: + if terminate_on_disconnect: + # If debuggee is still there, give it some time to terminate itself, + # then force-kill. Since the IDE is gone already, and nobody is waiting + # for us to respond, there's no rush. + debuggee.terminate(after=60) + class ServerMessages(Messages): """Message handlers and the associated global state for the server channel. """ - _channels = channels.Channels() + _only_allowed_while = Messages._only_allowed_while + + _shared = _Shared() _saved_messages = [] _hold_messages = True - # Socket was closed by the server. - def disconnect(self): - log.info("Debug server disconnected") - self._channels.close_server() - # Generic request handler, used if there's no specific handler below. def request(self, request): - return self._ide.delegate(request) + # Do not delegate requests from the server by default. There is a security + # boundary between the server and the adapter, and we cannot trust arbitrary + # requests sent over that boundary, since they may contain arbitrary code + # that the IDE will execute - e.g. "runInTerminal". The adapter must only + # propagate requests that it knows are safe. + request.isnt_valid("Requests from the debug server to the IDE are not allowed.") # Generic event handler, used if there's no specific handler below. def event(self, event): @@ -351,6 +383,53 @@ class ServerMessages(Messages): # also remove the 'initialized' event sent from IDE messages. pass + @_only_allowed_while("running") + def ptvsd_subprocess_event(self, event): + sub_pid = event("processId", int) + try: + debuggee.register_subprocess(sub_pid) + except Exception as exc: + event.cant_handle("{0}", exc) + self._ide.propagate(event) + + def terminated_event(self, event): + # Do not propagate this, since we'll report our own. + pass + + @_only_allowed_while("running") + def exited_event(self, event): + # For "launch", the adapter will report the event itself by observing the + # debuggee process directly, allowing the exit code to be captured more + # accurately. Thus, there's no need to propagate it in that case. + if self._shared.start_method == "attach": + self._ide.propagate(event) + + # Socket was closed by the server. + def disconnect(self): + log.info("Debug server disconnected.") + _channels.close_server() + + # In "launch", we must always report "exited", since we did not propagate it + # when the server reported it. In "attach", if the server disconnected without + # reporting "exited", we have no way to retrieve the exit code of the remote + # debuggee process - indeed, we don't even know if it exited or not. + report_exit = self._shared.start_method == "launch" + + try: + state.change("shutting_down") + except state.InvalidStateTransition: + # The IDE has either disconnected already, or requested "disconnect". + # There's no point reporting "exited" anymore. + report_exit = False + + if report_exit: + # The debuggee process should exit shortly after it has disconnected, but just + # in case it gets stuck, don't wait forever, and force-kill it if needed. + debuggee.terminate(after=5) + self._ide.send_event("exited", {"exitCode": debuggee.exit_code}) + + self._ide.send_event("terminated") + def release_events(self): # NOTE: This is temporary until debug server is updated to follow # DAP spec so we don't receive debugger events before configuration @@ -359,3 +438,6 @@ class ServerMessages(Messages): self._hold_messages = False for e in self._saved_messages: self._ide.propagate(e) + + +_channels = channels.Channels() diff --git a/src/ptvsd/adapter/state.py b/src/ptvsd/adapter/state.py index 65049166..ba99a221 100644 --- a/src/ptvsd/adapter/state.py +++ b/src/ptvsd/adapter/state.py @@ -7,7 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals """Tracks the overall state of the adapter, and enforces valid state transitions. """ -from ptvsd.common import log, singleton +from ptvsd.common import fmt, log, singleton # Order defines valid transitions. @@ -20,19 +20,32 @@ STATES = ( ) +class InvalidStateTransition(RuntimeError): + pass + + class State(singleton.ThreadSafeSingleton): _state = STATES[0] @property @singleton.autolocked_method def state(self): + """Returns the current state. + """ return self._state @state.setter @singleton.autolocked_method def state(self, new_state): - assert STATES.index(self._state) < STATES.index(new_state) - log.debug("Adapter state changed from {0!r} to {1!r}", self._state, new_state) + """Transitions to the new state, or raises InvalidStateTransition if the + state transition is not legal. + """ + state = self._state + if STATES.index(state) >= STATES.index(new_state): + raise InvalidStateTransition( + fmt("Cannot change adapter state from {0!r} to {1!r}", state, new_state) + ) + log.debug("Adapter state changed from {0!r} to {1!r}", state, new_state) self._state = new_state diff --git a/src/ptvsd/common/json.py b/src/ptvsd/common/json.py index fdbf31db..d050cee2 100644 --- a/src/ptvsd/common/json.py +++ b/src/ptvsd/common/json.py @@ -8,6 +8,7 @@ from __future__ import absolute_import, print_function, unicode_literals """ import json +import operator JsonDecoder = json.JSONDecoder @@ -74,3 +75,189 @@ class JsonObject(object): else: encoder = self.json_encoder return encoder.encode(self.value) + + +# JSON property validators, for use with MessageDict. +# +# A validator is invoked with the actual value of the JSON property passed to it as +# the sole argument; or if the property is missing in JSON, then () is passed. Note +# that None represents an actual null in JSON, while () is a missing value. +# +# The validator must either raise TypeError or ValueError describing why the property +# value is invalid, or else return the value of the property, possibly after performing +# some substitutions - e.g. replacing () with some default value. + + +def of_type(*classinfo, **kwargs): + """Returns a validator for a JSON property that requires it to have a value of + the specified type. If optional=True, () is also allowed. + + The meaning of classinfo is the same as for isinstance(). + """ + + assert len(classinfo) + optional = kwargs.pop("optional", False) + assert not len(kwargs) + + def validate(value): + if (optional and value == ()) or isinstance(value, classinfo): + return value + else: + raise TypeError("must be " + " or ".join(t.__name__ for t in classinfo)) + + return validate + + +def default(default): + """Returns a validator for a JSON property with a default value. + + The validator will only allow property values that have the same type as the + specified default value. + """ + + def validate(value): + if value == (): + return default + elif isinstance(value, type(default)): + return value + else: + raise TypeError("must be {0}".format(type(default).__name__)) + + return validate + + +def enum(*values, **kwargs): + """Returns a validator for a JSON enum. + + The validator will only allow the property to have one of the specified values. + + If optional=True, and the property is missing, the first value specified is used + as the default. + """ + + assert len(values) + optional = kwargs.pop("optional", False) + assert not len(kwargs) + + def validate(value): + if optional and value == (): + return values[0] + elif value in values: + return value + else: + raise ValueError("must be one of: {0!r}".format(list(values))) + + return validate + + +def array(validate_item=False, vectorize=False, size=None): + """Returns a validator for a JSON array. + + If the property is missing, it is treated as if it were []. Otherwise, it must + be a list. + + If validate_item=False, it's treated as if it were (lambda x: x) - i.e. any item + is considered valid, and is unchanged. If validate_item is a type or a tuple, + it's treated as if it were json.of_type(validate). + + Every item in the list is replaced with validate_item(item) in-place, propagating + any exceptions raised by the latter. If validate_item is a type or a tuple, it is + treated as if it were json.of_type(validate_item). + + If vectorize=True, and the value is neither a list nor a dict, it is treated as + if it were a single-element list containing that single value - e.g. "foo" is + then the same as ["foo"]; but {} is an error, and not [{}]. + + If size is not None, it can be an int, a tuple of one int, a tuple of two ints, + or a set. If it's an int, the array must have exactly that many elements. If it's + a tuple of one int, it's the minimum length. If it's a tuple of two ints, they + are the minimum and the maximum lengths. If it's a set, it's the set of sizes that + are valid - e.g. for {2, 4}, the array can be either 2 or 4 elements long. + """ + + if not validate_item: + validate_item = lambda x: x + elif isinstance(validate_item, type) or isinstance(validate_item, tuple): + validate_item = of_type(validate_item) + + if size is None: + validate_size = lambda _: True + elif isinstance(size, set): + size = {operator.index(n) for n in size} + validate_size = lambda value: ( + len(value) in size + or "must have {0} elements".format( + " or ".join(str(n) for n in sorted(size)) + ) + ) + elif isinstance(size, tuple): + assert 1 <= len(size) <= 2 + size = tuple(operator.index(n) for n in size) + min_len, max_len = (size + (None,))[0:2] + validate_size = lambda value: ( + "must have at least {0} elements".format(min_len) + if len(value) < min_len + else "must have at most {0} elements".format(max_len) + if max_len is not None and len(value) < max_len + else True + ) + else: + size = operator.index(size) + validate_size = lambda value: ( + len(value) == size or "must have {0} elements".format(size) + ) + + def validate(value): + if value == (): + value = [] + elif vectorize and not isinstance(value, (list, dict)): + value = [value] + + of_type(list)(value) + + size_err = validate_size(value) # True if valid, str if error + if size_err is not True: + raise ValueError(size_err) + + for i, item in enumerate(value): + try: + value[i] = validate_item(item) + except (TypeError, ValueError) as exc: + raise type(exc)("[{0!j}] {1}".format(i, exc)) + return value + + return validate + + +def object(validate_value=False): + """Returns a validator for a JSON object. + + If the property is missing, it is treated as if it were {}. Otherwise, it must + be a dict. + + If validate_value=False, it's treated as if it were (lambda x: x) - i.e. any + value is considered valid, and is unchanged. If validate_value is a type or a + tuple, it's treated as if it were json.of_type(validate_value). + + Every value in the dict is replaced with validate_value(value) in-place, propagating + any exceptions raised by the latter. If validate_value is a type or a tuple, it is + treated as if it were json.of_type(validate_value). Keys are not affected. + """ + + if isinstance(validate_value, type) or isinstance(validate_value, tuple): + validate_value = of_type(validate_value) + + def validate(value): + if value == (): + return {} + + of_type(dict)(value) + if validate_value: + for k, v in value.items(): + try: + value[k] = validate_value(v) + except (TypeError, ValueError) as exc: + raise type(exc)("[{0!j}] {1}".format(k, exc)) + return value + + return validate diff --git a/src/ptvsd/common/launcher.py b/src/ptvsd/common/launcher.py index 12c63606..eb875eba 100644 --- a/src/ptvsd/common/launcher.py +++ b/src/ptvsd/common/launcher.py @@ -4,10 +4,15 @@ from __future__ import absolute_import, print_function, unicode_literals +__all__ = ["main"] +import os.path import subprocess import sys +# Force absolute path on Python 2. +__file__ = os.path.abspath(__file__) + WAIT_ON_NORMAL_SWITCH = "--wait-on-normal" WAIT_ON_ABNORMAL_SWITCH = "--wait-on-abnormal" diff --git a/src/ptvsd/common/log.py b/src/ptvsd/common/log.py index e50172b0..37bd075f 100644 --- a/src/ptvsd/common/log.py +++ b/src/ptvsd/common/log.py @@ -6,6 +6,7 @@ from __future__ import print_function, absolute_import, unicode_literals import contextlib import functools +import inspect import io import platform import os @@ -158,10 +159,20 @@ def exception(format_string="", *args, **kwargs): if format_string: format_string += "\n\n" - format_string += "{exception}" + format_string += "{exception}\nStack where logged:\n{stack}" exception = "".join(traceback.format_exception(*exc_info)) - write_format(level, format_string, *args, exception=exception, **kwargs) + + f = inspect.currentframe() + f = f.f_back if f else f # don't log this frame + try: + stack = "".join(traceback.format_stack(f)) + finally: + del f # avoid cycles + + write_format( + level, format_string, *args, exception=exception, stack=stack, **kwargs + ) return exc_info[1] @@ -241,3 +252,18 @@ def suspend_handling(): yield finally: _tls.current_handler = what + + +# The following are helper shortcuts for printf debugging. They must never be used +# in production code. + + +def _repr(value): + warning("$REPR {0!r}", value) + + +def _vars(*names): + locals = inspect.currentframe().f_back.f_locals + if names: + locals = {name: locals[name] for name in names if name in locals} + warning("$VARS {0!r}", locals) diff --git a/src/ptvsd/common/messaging.py b/src/ptvsd/common/messaging.py index 2c3268e4..7cf3c29e 100644 --- a/src/ptvsd/common/messaging.py +++ b/src/ptvsd/common/messaging.py @@ -307,6 +307,46 @@ class MessageDict(collections.OrderedDict): def __repr__(self): return dict.__repr__(self) + def __call__(self, key, validate, optional=False): + """Like get(), but with validation. + + The item is first retrieved as if with self.get(key, default=()) - the default + value is () rather than None, so that JSON nulls are distinguishable from + missing properties. + + If optional=True, and the value is (), it's returned as is. Otherwise, the + item is validated by invoking validate(item) on it. + + If validate=False, it's treated as if it were (lambda x: x) - i.e. any value + is considered valid, and is returned unchanged. If validate is a type or a + tuple, it's treated as if it were json.of_type(validate). + + If validate() returns successfully, the item is substituted with the value + it returns - thus, the validator can e.g. replace () with a suitable default + value for the property. + + If validate() raises TypeError or ValueError, and self.message is not None, + __call__ raises InvalidMessageError that applies_to(self.message) with the + same text. If self.message is None, the exception is propagated as is. + + See ptvsd.common.json for reusable validators. + """ + + if not validate: + validate = lambda x: x + elif isinstance(validate, type) or isinstance(validate, tuple): + validate = json.of_type(validate) + + value = self.get(key, ()) + try: + value = validate(value) + except (TypeError, ValueError) as exc: + if self.message is None: + raise + else: + self.message.isnt_valid("{0!r} {1}", key, exc) + return value + def _invalid_if_no_key(func): def wrap(self, key, *args, **kwargs): try: @@ -346,6 +386,14 @@ class Message(object): """ raise NotImplementedError + def __call__(self, *args, **kwargs): + """Same as self.payload(...).""" + return self.payload(*args, **kwargs) + + def __contains__(self, key): + """Same as (key in self.payload).""" + return key in self.payload + def is_event(self, event=None): if not isinstance(self, Event): return False diff --git a/src/ptvsd/common/singleton.py b/src/ptvsd/common/singleton.py index 57a29c0a..328758e5 100644 --- a/src/ptvsd/common/singleton.py +++ b/src/ptvsd/common/singleton.py @@ -17,26 +17,87 @@ class Singleton(object): Concurrent invocations of T() from different threads are safe. """ - # All singletons share a single global construction lock, since we need to lock - # before we can construct any objects - it cannot be created per-class in __new__. - _lock = threading.RLock() + # A dual-lock scheme is necessary to be thread safe while avoiding deadlocks. + # _lock_lock is shared by all singleton types, and is used to construct their + # respective _lock instances when invoked for a new type. Then _lock is used + # to synchronize all further access for that type, including __init__. This way, + # __init__ for any given singleton can access another singleton, and not get + # deadlocked if that other singleton is trying to access it. + _lock_lock = threading.RLock() + _lock = None # Specific subclasses will get their own _instance set in __new__. _instance = None - def __new__(cls): + _is_shared = None # True if shared, False if exclusive + + def __new__(cls, *args, **kwargs): + # Allow arbitrary args and kwargs if shared=False, because that is guaranteed + # to construct a new singleton if it succeeds. Otherwise, this call might end + # up returning an existing instance, which might have been constructed with + # different arguments, so allowing them is misleading. + assert not kwargs.get("shared", False) or (len(args) + len(kwargs)) == 0, ( + "Cannot use constructor arguments when accessing a Singleton without " + "specifying shared=False." + ) + + # Avoid locking as much as possible with repeated double-checks - the most + # common path is when everything is already allocated. if not cls._instance: - with cls._lock: - if not cls._instance: - cls._instance = object.__new__(cls) + # If there's no per-type lock, allocate it. + if cls._lock is None: + with cls._lock_lock: + if cls._lock is None: + cls._lock = threading.RLock() + + # Now that we have a per-type lock, we can synchronize construction. + if not cls._instance: + with cls._lock: + if not cls._instance: + cls._instance = object.__new__(cls) + # To prevent having __init__ invoked multiple times, call + # it here directly, and then replace it with a stub that + # does nothing - that stub will get auto-invoked on return, + # and on all future singleton accesses. + cls._instance.__init__() + cls.__init__ = lambda *args, **kwargs: None + return cls._instance - def __init__(self): - """For singletons, __init__ is called on every access, not just on initial - creation. Initialization of attributes in derived classes should be done - on class level instead. + def __init__(self, *args, **kwargs): + """Initializes the singleton instance. Guaranteed to only be invoked once for + any given type derived from Singleton. + + If shared=False, the caller is requesting a singleton instance for their own + exclusive use. This is only allowed if the singleton has not been created yet; + if so, it is created and marked as being in exclusive use. While it is marked + as such, all attempts to obtain an existing instance of it immediately raise + an exception. The singleton can eventually be promoted to shared use by calling + share() on it. """ - pass + + shared = kwargs.pop("shared", True) + with self: + if shared: + assert ( + type(self)._is_shared is not False + ), "Cannot access a non-shared Singleton." + type(self)._is_shared = True + else: + assert type(self)._is_shared is None, "Singleton is already created." + + def __enter__(self): + """Lock this singleton to prevent concurrent access.""" + type(self)._lock.acquire() + return self + + def __exit__(self, exc_type, exc_value, exc_tb): + """Unlock this singleton to allow concurrent access.""" + type(self)._lock.release() + + def share(self): + """Share this singleton, if it was originally created with shared=False.""" + type(self)._is_shared = True class ThreadSafeSingleton(Singleton): @@ -54,18 +115,29 @@ class ThreadSafeSingleton(Singleton): on the same thread is also permitted. """ - # TODO: use a separate data lock for each subclass to reduce contention. + threadsafe_attrs = frozenset() + """Names of attributes that are guaranteed to be used in a thread-safe manner. - def __enter__(self): - type(self)._lock.acquire() - return self + This is typically used in conjunction with share() to simplify synchronization. + """ - def __exit__(self, exc_type, exc_value, exc_tb): - type(self)._lock.release() + readonly_attrs = frozenset() + """Names of attributes that are readonly. These can be read without locking, but + cannot be written at all. + + Every derived class gets its own separate set. Thus, for any given singleton type + T, an attribute can be made readonly after setting it, with T.readonly_attrs.add(). + """ + + def __init__(self, *args, **kwargs): + super(ThreadSafeSingleton, self).__init__(*args, **kwargs) + # Make sure each derived class gets a separate copy. + type(self).readonly_attrs = set(type(self).readonly_attrs) # Prevent callers from reading or writing attributes without locking, except for - # methods specifically marked @threadsafe_method. Such methods should perform - # the necessary locking to guarantee safety for the callers. + # reading attributes listed in threadsafe_attrs, and methods specifically marked + # with @threadsafe_method. Such methods should perform the necessary locking to + # ensure thread safety for the callers. @staticmethod def assert_locked(self): @@ -79,12 +151,15 @@ class ThreadSafeSingleton(Singleton): def __getattribute__(self, name): value = object.__getattribute__(self, name) - if not getattr(value, "is_threadsafe_method", False): - ThreadSafeSingleton.assert_locked(self) + if name not in (type(self).threadsafe_attrs | type(self).readonly_attrs): + if not getattr(value, "is_threadsafe_method", False): + ThreadSafeSingleton.assert_locked(self) return value def __setattr__(self, name, value): - ThreadSafeSingleton.assert_locked(self) + assert name not in type(self).readonly_attrs, "This attribute is read-only." + if name not in type(self).threadsafe_attrs: + ThreadSafeSingleton.assert_locked(self) return object.__setattr__(self, name, value) diff --git a/src/ptvsd/server/__main__.py b/src/ptvsd/server/__main__.py index c460123b..238aa7a6 100644 --- a/src/ptvsd/server/__main__.py +++ b/src/ptvsd/server/__main__.py @@ -17,6 +17,7 @@ import threading assert "pydevd" in sys.modules import pydevd +from ptvsd.common import log import ptvsd.server._remote import ptvsd.server.options import ptvsd.server.runner @@ -95,6 +96,14 @@ def set_log_dir(): return do +def set_log_stderr(): + + def do(arg, it): + log.stderr_levels |= set(log.LEVELS) + + return do + + def set_target(kind, parser=None): def do(arg, it): @@ -122,6 +131,7 @@ switches = [ ('--wait', None, set_true('wait'), False), ('--multiprocess', None, set_true('multiprocess'), False), ('--log-dir', '', set_log_dir(), False), + ('--log-stderr', None, set_log_stderr(), False), # Switches that are used internally by the IDE or ptvsd itself. ('--nodebug', None, set_nodebug, False),