Merge PR #433: fix(whatsapp): replace Linux-only fuser with cross-platform port cleanup
Authored by Farukest. Fixes #432. Extracts _kill_port_process() helper that uses netstat+taskkill on Windows and fuser on Linux. Previously, fuser calls were inline with bare except-pass, so on Windows orphaned bridge processes were never cleaned up — causing 'address already in use' errors on reconnect. Includes 5 tests covering both platforms, port matching edge cases, and exception suppression.
This commit is contained in:
commit
4a63737227
2 changed files with 130 additions and 22 deletions
|
|
@ -28,6 +28,41 @@ from typing import Dict, List, Optional, Any
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
def _kill_port_process(port: int) -> None:
|
||||||
|
"""Kill any process listening on the given TCP port."""
|
||||||
|
try:
|
||||||
|
if _IS_WINDOWS:
|
||||||
|
# Use netstat to find the PID bound to this port, then taskkill
|
||||||
|
result = subprocess.run(
|
||||||
|
["netstat", "-ano", "-p", "TCP"],
|
||||||
|
capture_output=True, text=True, timeout=5,
|
||||||
|
)
|
||||||
|
for line in result.stdout.splitlines():
|
||||||
|
parts = line.split()
|
||||||
|
if len(parts) >= 5 and parts[3] == "LISTENING":
|
||||||
|
local_addr = parts[1]
|
||||||
|
if local_addr.endswith(f":{port}"):
|
||||||
|
try:
|
||||||
|
subprocess.run(
|
||||||
|
["taskkill", "/PID", parts[4], "/F"],
|
||||||
|
capture_output=True, timeout=5,
|
||||||
|
)
|
||||||
|
except subprocess.SubprocessError:
|
||||||
|
pass
|
||||||
|
else:
|
||||||
|
result = subprocess.run(
|
||||||
|
["fuser", f"{port}/tcp"],
|
||||||
|
capture_output=True, timeout=5,
|
||||||
|
)
|
||||||
|
if result.returncode == 0:
|
||||||
|
subprocess.run(
|
||||||
|
["fuser", "-k", f"{port}/tcp"],
|
||||||
|
capture_output=True, timeout=5,
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
|
||||||
import sys
|
import sys
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parents[2]))
|
sys.path.insert(0, str(Path(__file__).resolve().parents[2]))
|
||||||
|
|
||||||
|
|
@ -145,21 +180,9 @@ class WhatsAppAdapter(BasePlatformAdapter):
|
||||||
self._session_path.mkdir(parents=True, exist_ok=True)
|
self._session_path.mkdir(parents=True, exist_ok=True)
|
||||||
|
|
||||||
# Kill any orphaned bridge from a previous gateway run
|
# Kill any orphaned bridge from a previous gateway run
|
||||||
try:
|
_kill_port_process(self._bridge_port)
|
||||||
result = subprocess.run(
|
import time
|
||||||
["fuser", f"{self._bridge_port}/tcp"],
|
time.sleep(1)
|
||||||
capture_output=True, timeout=5,
|
|
||||||
)
|
|
||||||
if result.returncode == 0:
|
|
||||||
# Port is in use — kill the process
|
|
||||||
subprocess.run(
|
|
||||||
["fuser", "-k", f"{self._bridge_port}/tcp"],
|
|
||||||
capture_output=True, timeout=5,
|
|
||||||
)
|
|
||||||
import time
|
|
||||||
time.sleep(2)
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# Start the bridge process in its own process group.
|
# Start the bridge process in its own process group.
|
||||||
# Route output to a log file so QR codes, errors, and reconnection
|
# Route output to a log file so QR codes, errors, and reconnection
|
||||||
|
|
@ -293,13 +316,7 @@ class WhatsAppAdapter(BasePlatformAdapter):
|
||||||
print(f"[{self.name}] Error stopping bridge: {e}")
|
print(f"[{self.name}] Error stopping bridge: {e}")
|
||||||
|
|
||||||
# Also kill any orphaned bridge processes on our port
|
# Also kill any orphaned bridge processes on our port
|
||||||
try:
|
_kill_port_process(self._bridge_port)
|
||||||
subprocess.run(
|
|
||||||
["fuser", "-k", f"{self._bridge_port}/tcp"],
|
|
||||||
capture_output=True, timeout=5,
|
|
||||||
)
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
self._running = False
|
self._running = False
|
||||||
self._bridge_process = None
|
self._bridge_process = None
|
||||||
|
|
|
||||||
|
|
@ -268,3 +268,94 @@ class TestFileHandleClosedOnError:
|
||||||
assert result is False
|
assert result is False
|
||||||
mock_fh.close.assert_called_once()
|
mock_fh.close.assert_called_once()
|
||||||
assert adapter._bridge_log_fh is None
|
assert adapter._bridge_log_fh is None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# _kill_port_process() cross-platform tests
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class TestKillPortProcess:
|
||||||
|
"""Verify _kill_port_process uses platform-appropriate commands."""
|
||||||
|
|
||||||
|
def test_uses_netstat_and_taskkill_on_windows(self):
|
||||||
|
from gateway.platforms.whatsapp import _kill_port_process
|
||||||
|
|
||||||
|
netstat_output = (
|
||||||
|
" Proto Local Address Foreign Address State PID\n"
|
||||||
|
" TCP 0.0.0.0:3000 0.0.0.0:0 LISTENING 12345\n"
|
||||||
|
" TCP 0.0.0.0:3001 0.0.0.0:0 LISTENING 99999\n"
|
||||||
|
)
|
||||||
|
mock_netstat = MagicMock(stdout=netstat_output)
|
||||||
|
mock_taskkill = MagicMock()
|
||||||
|
|
||||||
|
def run_side_effect(cmd, **kwargs):
|
||||||
|
if cmd[0] == "netstat":
|
||||||
|
return mock_netstat
|
||||||
|
if cmd[0] == "taskkill":
|
||||||
|
return mock_taskkill
|
||||||
|
return MagicMock()
|
||||||
|
|
||||||
|
with patch("gateway.platforms.whatsapp._IS_WINDOWS", True), \
|
||||||
|
patch("gateway.platforms.whatsapp.subprocess.run", side_effect=run_side_effect) as mock_run:
|
||||||
|
_kill_port_process(3000)
|
||||||
|
|
||||||
|
# netstat called
|
||||||
|
assert any(
|
||||||
|
call.args[0][0] == "netstat" for call in mock_run.call_args_list
|
||||||
|
)
|
||||||
|
# taskkill called with correct PID
|
||||||
|
assert any(
|
||||||
|
call.args[0] == ["taskkill", "/PID", "12345", "/F"]
|
||||||
|
for call in mock_run.call_args_list
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_does_not_kill_wrong_port_on_windows(self):
|
||||||
|
from gateway.platforms.whatsapp import _kill_port_process
|
||||||
|
|
||||||
|
netstat_output = (
|
||||||
|
" TCP 0.0.0.0:30000 0.0.0.0:0 LISTENING 55555\n"
|
||||||
|
)
|
||||||
|
mock_netstat = MagicMock(stdout=netstat_output)
|
||||||
|
|
||||||
|
with patch("gateway.platforms.whatsapp._IS_WINDOWS", True), \
|
||||||
|
patch("gateway.platforms.whatsapp.subprocess.run", return_value=mock_netstat) as mock_run:
|
||||||
|
_kill_port_process(3000)
|
||||||
|
|
||||||
|
# Should NOT call taskkill because port 30000 != 3000
|
||||||
|
assert not any(
|
||||||
|
call.args[0][0] == "taskkill"
|
||||||
|
for call in mock_run.call_args_list
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_uses_fuser_on_linux(self):
|
||||||
|
from gateway.platforms.whatsapp import _kill_port_process
|
||||||
|
|
||||||
|
mock_check = MagicMock(returncode=0)
|
||||||
|
|
||||||
|
with patch("gateway.platforms.whatsapp._IS_WINDOWS", False), \
|
||||||
|
patch("gateway.platforms.whatsapp.subprocess.run", return_value=mock_check) as mock_run:
|
||||||
|
_kill_port_process(3000)
|
||||||
|
|
||||||
|
calls = [c.args[0] for c in mock_run.call_args_list]
|
||||||
|
assert ["fuser", "3000/tcp"] in calls
|
||||||
|
assert ["fuser", "-k", "3000/tcp"] in calls
|
||||||
|
|
||||||
|
def test_skips_fuser_kill_when_port_free(self):
|
||||||
|
from gateway.platforms.whatsapp import _kill_port_process
|
||||||
|
|
||||||
|
mock_check = MagicMock(returncode=1) # port not in use
|
||||||
|
|
||||||
|
with patch("gateway.platforms.whatsapp._IS_WINDOWS", False), \
|
||||||
|
patch("gateway.platforms.whatsapp.subprocess.run", return_value=mock_check) as mock_run:
|
||||||
|
_kill_port_process(3000)
|
||||||
|
|
||||||
|
calls = [c.args[0] for c in mock_run.call_args_list]
|
||||||
|
assert ["fuser", "3000/tcp"] in calls
|
||||||
|
assert ["fuser", "-k", "3000/tcp"] not in calls
|
||||||
|
|
||||||
|
def test_suppresses_exceptions(self):
|
||||||
|
from gateway.platforms.whatsapp import _kill_port_process
|
||||||
|
|
||||||
|
with patch("gateway.platforms.whatsapp._IS_WINDOWS", True), \
|
||||||
|
patch("gateway.platforms.whatsapp.subprocess.run", side_effect=OSError("no netstat")):
|
||||||
|
_kill_port_process(3000) # must not raise
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue