From 9b6cec4622eea6c4a36f66682ce9ee9976d0210d Mon Sep 17 00:00:00 2001 From: Chris Sullivan Date: Mon, 28 Mar 2022 15:34:13 -0700 Subject: [PATCH 1/3] Only remove port forwarding applied in a session to avoid affecting global adb state. --- python/tvm/contrib/hexagon/build.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/python/tvm/contrib/hexagon/build.py b/python/tvm/contrib/hexagon/build.py index 16d3a30fd643..f0dfaabb0ef6 100644 --- a/python/tvm/contrib/hexagon/build.py +++ b/python/tvm/contrib/hexagon/build.py @@ -304,6 +304,7 @@ def __init__( self._serial_number = serial_number adb_socket = rpc_info["adb_server_socket"] if rpc_info["adb_server_socket"] else "tcp:5037" self._adb_device_sub_cmd = ["adb", "-L", adb_socket, "-s", self._serial_number] + self.forwarded_ports_ = [] super(HexagonLauncherAndroid, self).__init__(rpc_info, workspace) @@ -359,10 +360,6 @@ def _copy_binaries(self): def _run_server_script(self): """Setup the ADB connection and execute the server script.""" - # Removed pre-defined forward/reverse rules - subprocess.check_call(self._adb_device_sub_cmd + ["forward", "--remove-all"]) - subprocess.check_call(self._adb_device_sub_cmd + ["reverse", "--remove-all"]) - # Enable port reverse for RPC tracker rpc_tracker_port = self._rpc_info["rpc_tracker_port"] rpc_server_port = self._rpc_info["rpc_server_port"] @@ -372,9 +369,10 @@ def _run_server_script(self): ) # Enable port forward for RPC server. We forward 9 ports after the rpc_server_port. for i in range(0, 10): + port = rpc_server_port + i + self.forwarded_ports_.append(port) subprocess.check_call( - self._adb_device_sub_cmd - + ["forward", f"tcp:{rpc_server_port+i}", f"tcp:{rpc_server_port+i}"] + self._adb_device_sub_cmd + ["forward", f"tcp:{port}", f"tcp:{port}"] ) # Run server and connect to tracker @@ -385,6 +383,15 @@ def _run_server_script(self): stderr=subprocess.PIPE, ) + def _cleanup_port_forwarding(self): + # Removed pre-defined forward/reverse rules + rpc_tracker_port = self._rpc_info["rpc_tracker_port"] + subprocess.check_call( + self._adb_device_sub_cmd + ["reverse", "--remove", f"tcp:{rpc_tracker_port}"] + ) + for port in self.forwarded_ports_: + subprocess.check_call(self._adb_device_sub_cmd + ["forward", "--remove", f"tcp:{port}"]) + def start_server(self): """Abstract method implementation. See description in HexagonLauncherRPC.""" self._copy_binaries() @@ -392,6 +399,7 @@ def start_server(self): def stop_server(self): """Abstract method implementation. See description in HexagonLauncherRPC.""" + self._cleanup_port_forwarding() # Kill process children subprocess.Popen( self._adb_device_sub_cmd + ["shell", f"pkill -P `cat {self._workspace}/rpc_pid.txt`"] From 8782d4867b60adbf93863b6389a8826b0102ba50 Mon Sep 17 00:00:00 2001 From: Chris Sullivan Date: Mon, 28 Mar 2022 15:35:14 -0700 Subject: [PATCH 2/3] Send SIGINT to attempt to allow remote server to cleanup and undbind port in deconstruction --- python/tvm/contrib/hexagon/build.py | 30 +++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/python/tvm/contrib/hexagon/build.py b/python/tvm/contrib/hexagon/build.py index f0dfaabb0ef6..82d2be7f51d1 100644 --- a/python/tvm/contrib/hexagon/build.py +++ b/python/tvm/contrib/hexagon/build.py @@ -392,14 +392,18 @@ def _cleanup_port_forwarding(self): for port in self.forwarded_ports_: subprocess.check_call(self._adb_device_sub_cmd + ["forward", "--remove", f"tcp:{port}"]) - def start_server(self): - """Abstract method implementation. See description in HexagonLauncherRPC.""" - self._copy_binaries() - self._run_server_script() - - def stop_server(self): - """Abstract method implementation. See description in HexagonLauncherRPC.""" - self._cleanup_port_forwarding() + def _terminate_remote(self): + # Send interupt to main and child processes + subprocess.Popen( + self._adb_device_sub_cmd + + ["shell", f"pkill -l sigint -P `cat {self._workspace}/rpc_pid.txt`"] + ) + subprocess.Popen( + self._adb_device_sub_cmd + + ["shell", f"kill -s sigint `cat {self._workspace}/rpc_pid.txt`"] + ) + # Wait for processes to destruct cleanly after receiving the intrupt + subprocess.Popen(self._adb_device_sub_cmd + ["shell", "sleep", "0.1s"]) # Kill process children subprocess.Popen( self._adb_device_sub_cmd + ["shell", f"pkill -P `cat {self._workspace}/rpc_pid.txt`"] @@ -409,6 +413,16 @@ def stop_server(self): self._adb_device_sub_cmd + ["shell", f"kill `cat {self._workspace}/rpc_pid.txt`"] ) + def start_server(self): + """Abstract method implementation. See description in HexagonLauncherRPC.""" + self._copy_binaries() + self._run_server_script() + + def stop_server(self): + """Abstract method implementation. See description in HexagonLauncherRPC.""" + self._cleanup_port_forwarding() + self._terminate_remote() + class HexagonLauncherSimulator(HexagonLauncherRPC): """Hexagon Launcher for Hexagon simulator.""" From ed610c4de5d2208ba599c922f990a979127593aa Mon Sep 17 00:00:00 2001 From: Chris Sullivan Date: Tue, 12 Apr 2022 16:40:40 -0700 Subject: [PATCH 3/3] Only attempt to forward ports not in use by adb or the system. --- python/tvm/contrib/hexagon/build.py | 52 +++++++++++++++---- tests/python/contrib/test_hexagon/conftest.py | 6 +-- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/python/tvm/contrib/hexagon/build.py b/python/tvm/contrib/hexagon/build.py index 82d2be7f51d1..776faa9e9fd1 100644 --- a/python/tvm/contrib/hexagon/build.py +++ b/python/tvm/contrib/hexagon/build.py @@ -23,6 +23,7 @@ import os import pathlib import signal +import socket import stat import subprocess from typing import Union @@ -357,23 +358,46 @@ def _copy_binaries(self): for item in self.ANDROID_HEXAGON_RPC_FILES: self._copy_to_remote(lib_dir / item, self._workspace / item) + def _process_forwarded_ports(self): + forwarded_ports = subprocess.check_output(self._adb_device_sub_cmd + ["forward", "--list"]) + existing_forwards = [] + for forward in str(forwarded_ports).split("\\n"): + entry = forward.split() + if len(entry) == 3: + _, local, _ = entry + existing_forwards.append(int(local.strip("tcp:"))) + return existing_forwards + + def _forward_ports(self, rpc_server_port, existing_forwards): + # Enable port forward for RPC server. We forward the first ten open ports + # starting from the rpc_server_port + port = rpc_server_port + while len(self.forwarded_ports_) < 10: + if port not in existing_forwards and not _is_port_in_use(port): + subprocess.check_call( + self._adb_device_sub_cmd + ["forward", f"tcp:{port}", f"tcp:{port}"] + ) + self.forwarded_ports_.append(port) + port += 1 + + def _reverse_ports(self, rpc_tracker_port): + subprocess.check_call( + self._adb_device_sub_cmd + + ["reverse", f"tcp:{rpc_tracker_port}", f"tcp:{rpc_tracker_port}"] + ) + def _run_server_script(self): """Setup the ADB connection and execute the server script.""" + # Collect any existing adb port forwarding to avoid duplication + # with another running process + existing_forwards = self._process_forwarded_ports() # Enable port reverse for RPC tracker rpc_tracker_port = self._rpc_info["rpc_tracker_port"] rpc_server_port = self._rpc_info["rpc_server_port"] - subprocess.check_call( - self._adb_device_sub_cmd - + ["reverse", f"tcp:{rpc_tracker_port}", f"tcp:{rpc_tracker_port}"] - ) - # Enable port forward for RPC server. We forward 9 ports after the rpc_server_port. - for i in range(0, 10): - port = rpc_server_port + i - self.forwarded_ports_.append(port) - subprocess.check_call( - self._adb_device_sub_cmd + ["forward", f"tcp:{port}", f"tcp:{port}"] - ) + + self._reverse_ports(rpc_tracker_port) + self._forward_ports(rpc_server_port, existing_forwards) # Run server and connect to tracker subprocess.Popen( @@ -523,6 +547,12 @@ def stop_server(self): self._server_process.terminate() +# https://stackoverflow.com/a/52872579/2689797 +def _is_port_in_use(port: int) -> bool: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + return s.connect_ex(("localhost", port)) == 0 + + # pylint: disable=invalid-name def HexagonLauncher( serial_number: str, diff --git a/tests/python/contrib/test_hexagon/conftest.py b/tests/python/contrib/test_hexagon/conftest.py index 87bb69a34961..009150b1081c 100644 --- a/tests/python/contrib/test_hexagon/conftest.py +++ b/tests/python/contrib/test_hexagon/conftest.py @@ -85,10 +85,6 @@ def android_serial_number() -> Optional[str]: def get_free_port(): - # https://stackoverflow.com/a/52872579/2689797 - def is_port_in_use(port: int) -> bool: - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - return s.connect_ex(("localhost", port)) == 0 global previous_port if previous_port is None: @@ -96,7 +92,7 @@ def is_port_in_use(port: int) -> bool: else: port = previous_port + 1 - while is_port_in_use(port): + while tvm.contrib.hexagon.build._is_port_in_use(port): port = port + 1 if port < listen_port_max else listen_port_min previous_port = port