labath updated this revision to Diff 415120.
labath added a comment.
Herald added a project: All.

Remove just the forward-connect logic instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117559/new/

https://reviews.llvm.org/D117559

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py
  lldb/test/API/tools/lldb-server/TestPtyServer.py
  lldb/test/API/tools/lldb-server/commandline/TestGdbRemoteConnection.py

Index: lldb/test/API/tools/lldb-server/commandline/TestGdbRemoteConnection.py
===================================================================
--- lldb/test/API/tools/lldb-server/commandline/TestGdbRemoteConnection.py
+++ lldb/test/API/tools/lldb-server/commandline/TestGdbRemoteConnection.py
@@ -128,7 +128,6 @@
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @skipIfRemote  # reverse connect is not a supported use case for now
     def test_reverse_connect(self):
         # Reverse connect is the default connection method.
         self.connect_to_debug_monitor()
@@ -138,7 +137,7 @@
     @skipIfRemote
     def test_named_pipe(self):
         family, type, proto, _, addr = socket.getaddrinfo(
-            self.stub_hostname, 0, proto=socket.IPPROTO_TCP)[0]
+            "localhost", 0, proto=socket.IPPROTO_TCP)[0]
         self.sock = socket.socket(family, type, proto)
         self.sock.settimeout(self.DEFAULT_TIMEOUT)
 
@@ -149,11 +148,7 @@
         self.addTearDownHook(lambda: pipe.close())
 
         args = self.debug_monitor_extra_args
-        if lldb.remote_platform:
-            args += ["*:0"]
-        else:
-            args += ["localhost:0"]
-
+        args += ["localhost:0"]
         args += ["--named-pipe", pipe.name]
 
         server = self.spawnSubprocess(
@@ -170,3 +165,40 @@
 
         # Verify we can do the handshake.  If that works, we'll call it good.
         self.do_handshake()
+
+    @skipIfRemote
+    def test_forward_connect(self):
+        family, type, proto, _, addr = socket.getaddrinfo(
+            "localhost", 0, proto=socket.IPPROTO_TCP)[0]
+        self.sock = socket.socket(family, type, proto)
+        self.sock.settimeout(self.DEFAULT_TIMEOUT)
+
+        self.addTearDownHook(lambda: self.sock.close())
+
+        for i in range(11):
+            if i == 10:
+                self.fail("Failed to connect after 10 attempts")
+            args = [] + self.debug_monitor_extra_args
+            port = random.randint(5000, 65000)
+            addr = (addr[0], port)
+            args += ["[{}]:{}".format(*addr)]
+
+            server = self.spawnSubprocess(self.debug_monitor_exe, args,
+                    stderr=subprocess.PIPE)
+
+            try:
+                (_, err) = server.communicate(timeout=10)
+                # The server (unexpectedly) exited. If that happened because the
+                # port was taken, try again.
+                self.assertIn(b"failed to connect", err)
+                continue
+            except subprocess.TimeoutExpired:
+                # Good. The server is waiting for us to connect.
+                break
+        self.assertIsNotNone(server)
+
+        self.sock.connect(addr)
+        self._server = Server(self.sock, server)
+
+        # Verify we can do the handshake.  If that works, we'll call it good.
+        self.do_handshake()
Index: lldb/test/API/tools/lldb-server/TestPtyServer.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestPtyServer.py
+++ lldb/test/API/tools/lldb-server/TestPtyServer.py
@@ -8,6 +8,7 @@
 
 
 @skipIfWindows
+@skipIfRemote
 class PtyServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
     mydir = TestBase.compute_mydir(__file__)
 
@@ -20,21 +21,21 @@
         self._primary = io.FileIO(primary, 'r+b')
         self._secondary = io.FileIO(secondary, 'r+b')
 
-    def get_debug_monitor_command_line_args(self, attach_pid=None):
-        commandline_args = self.debug_monitor_extra_args
-        if attach_pid:
-            commandline_args += ["--attach=%d" % attach_pid]
+    def connect_to_debug_monitor(self, attach_pid=None):
+        self.assertIsNone(attach_pid)
 
+        # Create the command line.
         libc = ctypes.CDLL(None)
         libc.ptsname.argtypes = (ctypes.c_int,)
         libc.ptsname.restype = ctypes.c_char_p
         pty_path = libc.ptsname(self._primary.fileno()).decode()
-        commandline_args += ["serial://%s" % (pty_path,)]
-        return commandline_args
+        commandline_args = self.debug_monitor_extra_args + ["serial://%s" % (pty_path,)]
 
-    def connect_to_debug_monitor(self, attach_pid=None):
-        self.reverse_connect = False
-        server = self.launch_debug_monitor(attach_pid=attach_pid)
+        # Start the server.
+        server = self.spawnSubprocess(
+            self.debug_monitor_exe,
+            commandline_args,
+            install_remote=False)
         self.assertIsNotNone(server)
 
         # TODO: make it into proper abstraction
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py
@@ -4,10 +4,14 @@
 from lldbsuite.test.decorators import *
 from lldbgdbserverutils import *
 
+@skipIfRemote
 class GdbRemoteCompletionTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
     mydir = TestBase.compute_mydir(__file__)
 
     def init_lldb_server(self):
+        family, type, proto, _, addr = socket.getaddrinfo(
+            "localhost", 0, proto=socket.IPPROTO_TCP)[0]
+
         self.debug_monitor_exe = get_lldb_server_exe()
         if not self.debug_monitor_exe:
             self.skipTest("lldb-server exe not found")
@@ -15,7 +19,7 @@
         commandline_args = [
             "platform",
             "--listen",
-            "*:0",
+            "[{}]:{}".format(*addr),
             "--socket-file",
             port_file
         ]
@@ -24,9 +28,12 @@
             commandline_args,
             install_remote=False)
         self.assertIsNotNone(server)
-        self.stub_hostname = "localhost"
-        self.port = int(lldbutil.wait_for_file_on_target(self, port_file))
-        self.sock = self.create_socket()
+        port = int(lldbutil.wait_for_file_on_target(self, port_file))
+
+        self.sock = socket.socket(family, type, proto)
+        self.sock.settimeout(self.DEFAULT_TIMEOUT)
+        addr = (addr[0], port)
+        self.sock.connect(addr)
         self._server = Server(self.sock, server)
 
         self.do_handshake()
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -23,10 +23,6 @@
 import logging
 
 
-class _ConnectionRefused(IOError):
-    pass
-
-
 class GdbRemoteTestCaseFactory(type):
 
     def __new__(cls, name, bases, attrs):
@@ -133,23 +129,7 @@
 
         self.test_sequence = GdbRemoteTestSequence(self.logger)
         self.set_inferior_startup_launch()
-        self.port = self.get_next_port()
         self.stub_sends_two_stop_notifications_on_kill = False
-        if configuration.lldb_platform_url:
-            if configuration.lldb_platform_url.startswith('unix-'):
-                url_pattern = '(.+)://\[?(.+?)\]?/.*'
-            else:
-                url_pattern = '(.+)://(.+):\d+'
-            scheme, host = re.match(
-                url_pattern, configuration.lldb_platform_url).groups()
-            if configuration.lldb_platform_name == 'remote-android' and host != 'localhost':
-                self.stub_device = host
-                self.stub_hostname = 'localhost'
-            else:
-                self.stub_device = None
-                self.stub_hostname = host
-        else:
-            self.stub_hostname = "localhost"
 
         debug_server = self.getDebugServer()
         if debug_server == "debugserver":
@@ -183,19 +163,12 @@
             self.debug_monitor_extra_args = [
                 "--log-file=" + log_file, "--log-flags=0x800000"]
 
-    def get_next_port(self):
-        return 12000 + random.randint(0, 3999)
-
     def reset_test_sequence(self):
         self.test_sequence = GdbRemoteTestSequence(self.logger)
 
 
     def _init_llgs_test(self):
-        reverse_connect = True
         if lldb.remote_platform:
-            # Reverse connections may be tricky due to firewalls/NATs.
-            reverse_connect = False
-
             # FIXME: This is extremely linux-oriented
 
             # Grab the ppid from /proc/[shell pid]/stat
@@ -228,95 +201,14 @@
         self.debug_monitor_extra_args = ["gdbserver"]
         self.setUpServerLogging(is_llgs=True)
 
-        self.reverse_connect = reverse_connect
-
     def _init_debugserver_test(self):
         self.debug_monitor_exe = get_debugserver_exe()
         self.setUpServerLogging(is_llgs=False)
-        self.reverse_connect = True
 
         # The debugserver stub has a race on handling the 'k' command, so it sends an X09 right away, then sends the real X notification
         # when the process truly dies.
         self.stub_sends_two_stop_notifications_on_kill = True
 
-    def forward_adb_port(self, source, target, direction, device):
-        adb = ['adb'] + (['-s', device] if device else []) + [direction]
-
-        def remove_port_forward():
-            subprocess.call(adb + ["--remove", "tcp:%d" % source])
-
-        subprocess.call(adb + ["tcp:%d" % source, "tcp:%d" % target])
-        self.addTearDownHook(remove_port_forward)
-
-    def _verify_socket(self, sock):
-        # Normally, when the remote stub is not ready, we will get ECONNREFUSED during the
-        # connect() attempt. However, due to the way how ADB forwarding works, on android targets
-        # the connect() will always be successful, but the connection will be immediately dropped
-        # if ADB could not connect on the remote side. This function tries to detect this
-        # situation, and report it as "connection refused" so that the upper layers attempt the
-        # connection again.
-        triple = self.dbg.GetSelectedPlatform().GetTriple()
-        if not re.match(".*-.*-.*-android", triple):
-            return  # Not android.
-        can_read, _, _ = select.select([sock], [], [], 0.1)
-        if sock not in can_read:
-            return  # Data is not available, but the connection is alive.
-        if len(sock.recv(1, socket.MSG_PEEK)) == 0:
-            raise _ConnectionRefused()  # Got EOF, connection dropped.
-
-    def create_socket(self):
-        try:
-            sock = socket.socket(family=socket.AF_INET)
-        except OSError as e:
-            if e.errno != errno.EAFNOSUPPORT:
-                raise
-            sock = socket.socket(family=socket.AF_INET6)
-
-        logger = self.logger
-
-        triple = self.dbg.GetSelectedPlatform().GetTriple()
-        if re.match(".*-.*-.*-android", triple):
-            self.forward_adb_port(
-                self.port,
-                self.port,
-                "forward",
-                self.stub_device)
-
-        logger.info(
-            "Connecting to debug monitor on %s:%d",
-            self.stub_hostname,
-            self.port)
-        connect_info = (self.stub_hostname, self.port)
-        try:
-            sock.connect(connect_info)
-        except socket.error as serr:
-            if serr.errno == errno.ECONNREFUSED:
-                raise _ConnectionRefused()
-            raise serr
-
-        def shutdown_socket():
-            if sock:
-                try:
-                    # send the kill packet so lldb-server shuts down gracefully
-                    sock.sendall(GdbRemoteTestCaseBase._GDBREMOTE_KILL_PACKET)
-                except:
-                    logger.warning(
-                        "failed to send kill packet to debug monitor: {}; ignoring".format(
-                            sys.exc_info()[0]))
-
-                try:
-                    sock.close()
-                except:
-                    logger.warning(
-                        "failed to close socket to debug monitor: {}; ignoring".format(
-                            sys.exc_info()[0]))
-
-        self.addTearDownHook(shutdown_socket)
-
-        self._verify_socket(sock)
-
-        return sock
-
     def set_inferior_startup_launch(self):
         self._inferior_startup = self._STARTUP_LAUNCH
 
@@ -330,14 +222,7 @@
         commandline_args = self.debug_monitor_extra_args
         if attach_pid:
             commandline_args += ["--attach=%d" % attach_pid]
-        if self.reverse_connect:
-            commandline_args += ["--reverse-connect", self.connect_address]
-        else:
-            if lldb.remote_platform:
-                commandline_args += ["*:{}".format(self.port)]
-            else:
-                commandline_args += ["localhost:{}".format(self.port)]
-
+        commandline_args += ["--reverse-connect", self.connect_address]
         return commandline_args
 
     def get_target_byte_order(self):
@@ -346,16 +231,14 @@
         return target.GetByteOrder()
 
     def launch_debug_monitor(self, attach_pid=None, logfile=None):
-        if self.reverse_connect:
-            family, type, proto, _, addr = socket.getaddrinfo("localhost", 0, proto=socket.IPPROTO_TCP)[0]
-            sock = socket.socket(family, type, proto)
-            sock.settimeout(self.DEFAULT_TIMEOUT)
-
-            sock.bind(addr)
-            sock.listen(1)
-            addr = sock.getsockname()
-            self.connect_address = "[{}]:{}".format(*addr)
+        family, type, proto, _, addr = socket.getaddrinfo("localhost", 0, proto=socket.IPPROTO_TCP)[0]
+        sock = socket.socket(family, type, proto)
+        sock.settimeout(self.DEFAULT_TIMEOUT)
 
+        sock.bind(addr)
+        sock.listen(1)
+        addr = sock.getsockname()
+        self.connect_address = "[{}]:{}".format(*addr)
 
         # Create the command line.
         commandline_args = self.get_debug_monitor_command_line_args(
@@ -368,70 +251,21 @@
             install_remote=False)
         self.assertIsNotNone(server)
 
-        if self.reverse_connect:
-            self.sock = sock.accept()[0]
-            self.sock.settimeout(self.DEFAULT_TIMEOUT)
+        self.sock = sock.accept()[0]
+        self.sock.settimeout(self.DEFAULT_TIMEOUT)
 
         return server
 
     def connect_to_debug_monitor(self, attach_pid=None):
-        if self.reverse_connect:
-            # Create the stub.
-            server = self.launch_debug_monitor(attach_pid=attach_pid)
-            self.assertIsNotNone(server)
-
-            # Schedule debug monitor to be shut down during teardown.
-            logger = self.logger
-
-            self._server = Server(self.sock, server)
-            return server
-
-        # We're using a random port algorithm to try not to collide with other ports,
-        # and retry a max # times.
-        attempts = 0
-        MAX_ATTEMPTS = 20
-
-        while attempts < MAX_ATTEMPTS:
-            server = self.launch_debug_monitor(attach_pid=attach_pid)
-
-            # Schedule debug monitor to be shut down during teardown.
-            logger = self.logger
-
-            connect_attemps = 0
-            MAX_CONNECT_ATTEMPTS = 10
-
-            while connect_attemps < MAX_CONNECT_ATTEMPTS:
-                # Create a socket to talk to the server
-                try:
-                    logger.info("Connect attempt %d", connect_attemps + 1)
-                    self.sock = self.create_socket()
-                    self._server = Server(self.sock, server)
-                    return server
-                except _ConnectionRefused as serr:
-                    # Ignore, and try again.
-                    pass
-                time.sleep(0.5)
-                connect_attemps += 1
-
-            # We should close the server here to be safe.
-            server.terminate()
-
-            # Increment attempts.
-            print(
-                "connect to debug monitor on port %d failed, attempt #%d of %d" %
-                (self.port, attempts + 1, MAX_ATTEMPTS))
-            attempts += 1
-
-            # And wait a random length of time before next attempt, to avoid
-            # collisions.
-            time.sleep(random.randint(1, 5))
-
-            # Now grab a new port number.
-            self.port = self.get_next_port()
-
-        raise Exception(
-            "failed to create a socket to the launched debug monitor after %d tries" %
-            attempts)
+        # Create the stub.
+        server = self.launch_debug_monitor(attach_pid=attach_pid)
+        self.assertIsNotNone(server)
+
+        # Schedule debug monitor to be shut down during teardown.
+        logger = self.logger
+
+        self._server = Server(self.sock, server)
+        return server
 
     def launch_process_for_attach(
             self,
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -387,7 +387,7 @@
     def pid(self):
         return self._proc.pid
 
-    def launch(self, executable, args, extra_env):
+    def launch(self, executable, args, extra_env, stderr=None):
         env=None
         if extra_env:
             env = dict(os.environ)
@@ -395,9 +395,9 @@
 
         self._proc = Popen(
             [executable] + args,
-            stdout=open(
-                os.devnull) if not self._trace_on else None,
+            stdout=open(os.devnull) if not self._trace_on else None,
             stdin=PIPE,
+            stderr=stderr,
             preexec_fn=lldbplatformutil.enable_attach,
             env=env)
 
@@ -430,6 +430,9 @@
     def wait(self, timeout=None):
         return self._proc.wait(timeout)
 
+    def communicate(self, timeout=None):
+        return self._proc.communicate(timeout=timeout)
+
 
 class _RemoteProcess(_BaseProcess):
 
@@ -891,13 +894,14 @@
             del p
         del self.subprocesses[:]
 
-    def spawnSubprocess(self, executable, args=[], extra_env=None, install_remote=True):
+    def spawnSubprocess(self, executable, args=[], extra_env=None,
+            install_remote=True, **kwargs):
         """ Creates a subprocess.Popen object with the specified executable and arguments,
             saves it in self.subprocesses, and returns the object.
         """
         proc = _RemoteProcess(
             install_remote) if lldb.remote_platform else _LocalProcess(self.TraceOn())
-        proc.launch(executable, args, extra_env=extra_env)
+        proc.launch(executable, args, extra_env=extra_env, **kwargs)
         self.subprocesses.append(proc)
         return proc
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to