fix: release hdc fport so the local port is free after each run

hmdriver2/_client.py

@@ -1,13 +1,14 @@
 # -*- coding: utf-8 -*-
+import atexit
 import socket
 import json
+import sys
 import time
 import os
 import hashlib
 import typing
 from typing import Optional
 from datetime import datetime
-from functools import cached_property
 
 from . import logger
 from .hdc import HdcWrapper
@@ -24,17 +25,40 @@ class HmClient:
     def __init__(self, serial: str):
         self.hdc = HdcWrapper(serial)
         self.sock = None
+        self._local_port = None
+        self._released = False
+        self._atexit_registered = False
 
-    @cached_property
+    @property
     def local_port(self):
-        fports = self.hdc.list_fport()
-        logger.debug(fports) if fports else None
-
-        return self.hdc.forward_port(UITEST_SERVICE_PORT)
+        if self._local_port is None:
+            fports = self.hdc.list_fport()
+            logger.debug(fports) if fports else None
+            self._local_port = self.hdc.forward_port(UITEST_SERVICE_PORT)
+            self._register_atexit()
+        return self._local_port
+
+    def _register_atexit(self):
+        if self._atexit_registered:
+            return
+        atexit.register(self.release)
+        self._atexit_registered = True
 
     def _rm_local_port(self):
+        """Remove the HDC forward opened by this client.
+
+        Must not read :attr:`local_port` when it was never allocated: that
+        property opens a new ``hdc fport`` and would leak another mapping.
+        """
+        port = self._local_port
+        if not port:
+            logger.debug("rm fport skipped: no local port allocated")
+            return
         logger.debug("rm fport local port")
-        self.hdc.rm_forward(self.local_port, UITEST_SERVICE_PORT)
+        try:
+            self.hdc.rm_forward(port, UITEST_SERVICE_PORT)
+        finally:
+            self._local_port = None
 
     def _connect_sock(self):
         """Create socket and connect to the uiTEST server."""
@@ -140,23 +164,57 @@ def invoke_captures(self, api: str, args: typing.List = []) -> HypiumResponse:
 
     def start(self):
         logger.info("Start HmClient connection")
+        self._released = False
         _UITestService(self.hdc).init()
 
         self._connect_sock()
 
         self._create_hdriver()
 
+    def __enter__(self):
+        self.start()
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.release()
+
     def release(self):
+        """Close the socket and remove the local HDC port forward.
+
+        Idempotent. Safe from ``Driver.close``, ``__exit__``, ``atexit``, and
+        ``__del__``. During interpreter finalization, ``hdc fport rm`` is
+        skipped (subprocess is unsafe then); atexit / explicit close run
+        earlier so the forward is still removed on a normal exit.
+        """
+        if self._released:
+            return
+        self._released = True
+
         logger.info(f"Release {self.__class__.__name__} connection")
+
         try:
             if self.sock:
+                try:
+                    self.sock.shutdown(socket.SHUT_RDWR)
+                except OSError:
+                    pass
                 self.sock.close()
                 self.sock = None
 
-            self._rm_local_port()
+            if getattr(sys, "is_finalizing", lambda: False)():
+                logger.debug("release: interpreter finalizing, skipped fport rm")
+                return
 
+            self._rm_local_port()
         except Exception as e:
             logger.error(f"An error occurred: {e}")
+        finally:
+            if self._atexit_registered:
+                try:
+                    atexit.unregister(self.release)
+                except Exception:
+                    pass
+                self._atexit_registered = False
 
     def _create_hdriver(self) -> DriverData:
         logger.debug("Create uitest driver")

hmdriver2/driver.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 
+import atexit
 import json
 import uuid
 import re
@@ -47,8 +48,10 @@ def __init__(self, serial: Optional[str] = None):
         self.serial = serial
         self._client = HmClient(self.serial)
         self.hdc = self._client.hdc
+        self._closed = False
         self._init_hmclient()
         self._initialized = True  # Mark the instance as initialized
+        atexit.register(self.close)
         del self._serial_for_init  # Clean up temporary attribute
 
     @classmethod
@@ -71,10 +74,47 @@ def __call__(self, **kwargs) -> UiObject:
 
         return UiObject(self._client, **kwargs)
 
-    def __del__(self):
-        Driver._instance.clear()
-        if hasattr(self, '_client') and self._client:
+    def _unregister_singleton_if_self(self) -> None:
+        """Drop this serial from the singleton map only when the slot is us."""
+        serial = getattr(self, "serial", None)
+        if serial is None:
+            return
+        if Driver._instance.get(serial) is self:
+            Driver._instance.pop(serial, None)
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.close()
+
+    def close(self) -> None:
+        """Release the Hypium socket and ``hdc fport`` mapping.
+
+        Idempotent. Call this (or use ``with Driver()`` / ``stop()``) so the
+        forwarded local port is freed before the next run. ``__del__`` alone
+        is not reliable; ``atexit`` also invokes this on process exit.
+        """
+        if getattr(self, "_closed", False):
+            return
+        self._closed = True
+        try:
+            atexit.unregister(self.close)
+        except Exception:
+            pass
+        if hasattr(self, "_client") and self._client:
             self._client.release()
+        self._unregister_singleton_if_self()
+
+    def stop(self) -> None:
+        """Alias for :meth:`close`."""
+        self.close()
+
+    def __del__(self):
+        try:
+            self.close()
+        except Exception:
+            pass
 
     def _init_hmclient(self):
         self._client.start()

hmdriver2/hdc.py

@@ -5,6 +5,7 @@
 import shlex
 import re
 import os
+import signal
 import subprocess
 from typing import Union, List, Dict, Tuple, Optional
 
@@ -14,17 +15,45 @@
 from .exception import HdcError, DeviceNotFoundError
 
 
-def _execute_command(cmdargs: Union[str, List[str]]) -> CommandResult:
+# hdc fport rm can block indefinitely (issues #47, #54). Bound teardown so
+# driver close / atexit can finish and the local port is eligible for reuse.
+FPORT_RM_TIMEOUT = 5
+
+
+def _execute_command(cmdargs: Union[str, List[str]],
+                     timeout: Optional[float] = None) -> CommandResult:
     if isinstance(cmdargs, (list, tuple)):
         cmdline: str = ' '.join(list(map(shlex.quote, cmdargs)))
     elif isinstance(cmdargs, str):
         cmdline = cmdargs
 
     logger.debug(cmdline)
     try:
-        process = subprocess.Popen(cmdline, stdout=subprocess.PIPE,
-                                   stderr=subprocess.PIPE, shell=True)
-        output, error = process.communicate()
+        popen_kwargs = dict(
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            shell=True,
+        )
+        # Isolate timed commands so a hung hdc child (fport rm) can be killed
+        # without waiting for grandchildren after the shell exits.
+        if timeout is not None:
+            if os.name == "nt":
+                popen_kwargs["creationflags"] = getattr(
+                    subprocess, "CREATE_NEW_PROCESS_GROUP", 0)
+            else:
+                popen_kwargs["start_new_session"] = True
+
+        process = subprocess.Popen(cmdline, **popen_kwargs)
+        try:
+            output, error = process.communicate(timeout=timeout)
+        except subprocess.TimeoutExpired:
+            _kill_process_group(process)
+            try:
+                process.communicate(timeout=1)
+            except Exception:
+                pass
+            return CommandResult("", "timeout expired", -1)
+
         output = output.decode('utf-8')
         error = error.decode('utf-8')
         exit_code = process.returncode
@@ -37,6 +66,19 @@ def _execute_command(cmdargs: Union[str, List[str]]) -> CommandResult:
         return CommandResult("", str(e), -1)
 
 
+def _kill_process_group(process: subprocess.Popen) -> None:
+    try:
+        if os.name == "nt":
+            process.kill()
+        else:
+            os.killpg(process.pid, signal.SIGKILL)
+    except Exception:
+        try:
+            process.kill()
+        except Exception:
+            pass
+
+
 def _build_hdc_prefix() -> str:
     """
     Construct the hdc command prefix based on environment variables.
@@ -86,7 +128,9 @@ def forward_port(self, rport: int) -> int:
         return lport
 
     def rm_forward(self, lport: int, rport: int) -> int:
-        result = _execute_command(f"{self.hdc_prefix} -t {self.serial} fport rm tcp:{lport} tcp:{rport}")
+        result = _execute_command(
+            f"{self.hdc_prefix} -t {self.serial} fport rm tcp:{lport} tcp:{rport}",
+            timeout=FPORT_RM_TIMEOUT)
         if result.exit_code != 0:
             raise HdcError("HDC rm forward error", result.error)
         return lport