Reverse the connection direction used by enable attach internals. (#1833)

* Minor cleanup
* Reverse the direction used by enable attach internals.
* Handle occasional Invalid argument

.sonarcloud.properties

@@ -1,5 +1,5 @@
 sonar.sources=src/ptvsd
-sonar.exclusions=src/ptvsd/_vendored/**/*,pytests/**/*,src/ptvsd/_version.py
+sonar.exclusions=src/ptvsd/_vendored/**/*,src/ptvsd/_version.py
 sonar.cfamily.build-wrapper-output.bypass=true
 sonar.c.file.suffixes=-
 sonar.cpp.file.suffixes=-

sonar-project.properties

@@ -1,5 +1,5 @@
 sonar.sources=src/ptvsd
-sonar.exclusions=src/ptvsd/_vendored/**/*,pytests/**/*,src/ptvsd/_version.py
+sonar.exclusions=src/ptvsd/_vendored/**/*,src/ptvsd/_version.py
 sonar.cfamily.build-wrapper-output.bypass=true
 sonar.c.file.suffixes=-
 sonar.cpp.file.suffixes=-

src/ptvsd/__init__.py

@@ -17,6 +17,7 @@ __all__ = [
     "enable_attach",
     "is_attached",
     "wait_for_attach",
+    "tracing",
 ]
 
 import codecs

src/ptvsd/_vendored/pydevd/pydevd.py

@@ -2253,6 +2253,7 @@ def _enable_attach(
     patch_multiprocessing=False,
     access_token=None,
     ide_access_token=None,
+    as_client=False
     ):
     '''
     Starts accepting connections at the given host/port. The debugger will not be initialized nor
@@ -2275,16 +2276,19 @@ def _enable_attach(
         port=port,
         suspend=False,
         wait_for_ready_to_run=False,
-        block_until_connected=False,
+        block_until_connected=as_client,
         dont_trace_start_patterns=dont_trace_start_patterns,
         dont_trace_end_paterns=dont_trace_end_paterns,
         patch_multiprocessing=patch_multiprocessing,
         access_token=access_token,
         ide_access_token=ide_access_token,
     )
-    py_db = get_global_debugger()
-    py_db.wait_for_server_socket_ready()
-    return py_db._server_socket_name
+
+    if not as_client:
+        py_db = get_global_debugger()
+        py_db.wait_for_server_socket_ready()
+        return py_db._server_socket_name
+    return (host, port)
 
 
 def _wait_for_attach(cancel=None):

src/ptvsd/adapter/__main__.py

@@ -5,6 +5,7 @@
 from __future__ import absolute_import, print_function, unicode_literals
 
 import argparse
+import json
 import locale
 import os
 import sys
@@ -33,11 +34,25 @@ def main(args):
     if args.port is None:
         session.connect_to_ide()
     else:
-        if args.for_server_on_port is not None:
-            session.connect_to_server(("127.0.0.1", args.for_server_on_port))
-        with session.accept_connection_from_ide((args.host, args.port)) as (_, port):
-            if session.server:
-                session.server.set_debugger_property({"adapterPort": port})
+        if args.for_enable_attach:
+            # Users may want the adapter to choose the port for them, by setting port==0.
+            # For example, the Python Data Science extension uses this mode in enable_attach.
+            # Let enable_attach know the port that users should use to connect to the adapter.
+            with session.accept_connection_from_ide((args.host, args.port)) as (adapter_host, adapter_port):
+                # This mode is used only for enable_attach. Here, we always connect to
+                # adapter from the debug server as client. Adapter needs to start a listener
+                # and provide that port to debug server.
+                with session.accept_connection_from_server() as (server_host, server_port):
+                    connection_details = {
+                        "adapter": {"host": adapter_host, "port": adapter_port},
+                        "server": {"host": server_host, "port": server_port}
+                    }
+                    log.info("Writing to stdout for enable_attach: {0!r}", connection_details)
+                    print(json.dumps(connection_details))
+                    sys.stdout.flush()
+        else:
+            with session.accept_connection_from_ide((args.host, args.port)) as (_, adapter_port):
+                pass
     session.wait_for_completion()
 
 
@@ -61,10 +76,8 @@ def _parse_argv(argv):
     )
 
     parser.add_argument(
-        "--for-server-on-port",
-        type=int,
-        default=None,
-        metavar="PORT",
+        "--for-enable-attach",
+        action="store_true",
         help=argparse.SUPPRESS,
     )
 

src/ptvsd/server/api.py

@@ -14,12 +14,12 @@ import threading
 import ptvsd
 from ptvsd.common import log, options as common_opts
 from ptvsd.server import options as server_opts
-from ptvsd.common.compat import queue
 from _pydevd_bundle.pydevd_constants import get_global_debugger
 from pydevd_file_utils import get_abs_path_real_path_and_base_from_file
 
 
 _QUEUE_TIMEOUT = 10
+_ADAPTER_PATH = os.path.join(os.path.dirname(ptvsd.__file__), "adapter")
 
 
 def wait_for_attach():
@@ -80,56 +80,53 @@ def enable_attach(dont_trace_start_patterns, dont_trace_end_patterns):
     if hasattr(enable_attach, "called"):
         raise RuntimeError("enable_attach() can only be called once per process.")
 
-    host, port = pydevd._enable_attach(
-        ("127.0.0.1", 0),
+    import subprocess
+    adapter_args = [
+        sys.executable,
+        _ADAPTER_PATH,
+        "--host",
+        server_opts.host,
+        "--port",
+        str(server_opts.port),
+        "--for-enable-attach",
+    ]
+
+    if common_opts.log_dir is not None:
+        adapter_args += ["--log-dir", common_opts.log_dir]
+
+    log.info("enable_attach() spawning adapter: {0!r}", adapter_args)
+
+    # Adapter life time is expected to be longer than this process,
+    # so never wait on the adapter process
+    process = subprocess.Popen(
+        adapter_args,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+    )
+
+    line = process.stdout.readline()
+    if isinstance(line, bytes):
+        line = line.decode("utf-8")
+    connection_details = json.JSONDecoder().decode(line)
+    log.info("Connection details received from adapter: {0!r}", connection_details)
+
+    host = "127.0.0.1" # This should always be loopback address.
+    port = connection_details["server"]["port"]
+    address = ("127.0.0.1", port)
+
+    pydevd._enable_attach(
+        address,
         dont_trace_start_patterns=dont_trace_start_patterns,
         dont_trace_end_paterns=dont_trace_end_patterns,
         patch_multiprocessing=server_opts.multiprocess,
+        as_client=True,
     )
 
-    log.info("pydevd debug server running at: {0}:{1}", host, port)
+    log.info("pydevd debug client connected to: {0}:{1}", host, port)
 
-    class _DAPMessagesListener(pydevd.IDAPMessagesListener):
-        def before_send(self, msg):
-            pass
-
-        def after_receive(self, msg):
-            try:
-                if msg["command"] == "setDebuggerProperty":
-                    port_queue.put(msg["arguments"]["adapterPort"])
-            except KeyError:
-                pass
-
-    port_queue = queue.Queue()
-    pydevd.add_dap_messages_listener(_DAPMessagesListener())
-
-    with pydevd.skip_subprocess_arg_patch():
-        import subprocess
-
-        adapter_args = [
-            sys.executable,
-            os.path.join(os.path.dirname(ptvsd.__file__), "adapter"),
-            "--host",
-            server_opts.host,
-            "--port",
-            str(server_opts.port),
-            "--for-server-on-port",
-            str(port),
-        ]
-
-        if common_opts.log_dir is not None:
-            adapter_args += ["--log-dir", common_opts.log_dir]
-
-        log.info("enable_attach() spawning adapter: {0!r}", adapter_args)
-
-        # Adapter life time is expected to be longer than this process,
-        # so never wait on the adapter process
-        process = subprocess.Popen(adapter_args, bufsize=0)
-        # Ensure that we ignore the adapter process when terminating the
-        # debugger.
-        pydevd.add_dont_terminate_child_pid(process.pid)
-
-    server_opts.port = port_queue.get(True, _QUEUE_TIMEOUT)
+    # Ensure that we ignore the adapter process when terminating the debugger.
+    pydevd.add_dont_terminate_child_pid(process.pid)
+    server_opts.port =  connection_details["adapter"]["port"]
 
     listener_file = os.getenv("PTVSD_LISTENER_FILE")
     if listener_file is not None:

tests/debug/comms.py

@@ -9,7 +9,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 import threading
 import socket
 
-from ptvsd.common import fmt, log, messaging
+from ptvsd.common import fmt, log, messaging, sockets
 from tests.timeline import Request, Response
 
 
@@ -27,9 +27,7 @@ class BackChannel(object):
         return fmt("{0}.backchannel", self.session.debuggee_id)
 
     def listen(self):
-        self._server_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-        self._server_socket.settimeout(self.TIMEOUT)
-        self._server_socket.bind(("127.0.0.1", 0))
+        self._server_socket = sockets.create_server("127.0.0.1", 0, self.TIMEOUT)
         _, self.port = self._server_socket.getsockname()
         self._server_socket.listen(0)