Skip to content

Commit f838387

Browse files
committed
fix(net): fix port check arg order, add detailed GDB conflict messages
Fix is_port_available(host, port) parameter order to match original check_port_available(host, port) API in main.py. The reversed order caused the port check to silently pass, letting Flask report the conflict with a generic message instead of our detailed diagnostics. GDB manager now shows process name, PID, command, and kill hint when port is occupied, matching main.py's UX. Fix flake8 F541 (f-string without placeholder). Signed-off-by: VIFEX <vifextech@foxmail.com>
1 parent 53b3b40 commit f838387

4 files changed

Lines changed: 74 additions & 28 deletions

File tree

Tools/WebServer/core/gdb_manager.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from core.gdb_bridge import GDBRSPBridge
2020
from core.gdb_session import GDBSession
2121
from core.state import ToolLogHandler
22-
from utils.net import check_and_free_port
22+
from utils.net import get_port_owner, is_port_available, kill_port_owner
2323

2424
logger = logging.getLogger(__name__)
2525

@@ -207,9 +207,22 @@ def start_external_gdb_server(state, read_memory_fn=None, write_memory_fn=None)
207207
logger.info("[ExtGDB] Using provided memory callbacks (may be offline stubs)")
208208

209209
try:
210-
if not check_and_free_port(port):
211-
logger.error(f"Cannot start external GDB server: port {port} is occupied")
212-
return False
210+
if not is_port_available("127.0.0.1", port):
211+
owner = get_port_owner(port)
212+
if owner:
213+
logger.error(f"❌ GDB server port {port} is already in use!")
214+
logger.error(f" Process: {owner['name']} (PID {owner['pid']})")
215+
logger.error(f" Command: {owner['cmdline']}")
216+
logger.warning(f" Killing stale process PID {owner['pid']}...")
217+
else:
218+
logger.error(f"❌ GDB server port {port} is in use by unknown process")
219+
220+
if not kill_port_owner(port):
221+
logger.error(f" Failed to free port {port}. Options:")
222+
if owner:
223+
logger.error(f" kill {owner['pid']}")
224+
logger.error(" Change port in config (external_gdb_port)")
225+
return False
213226

214227
bridge = GDBRSPBridge(
215228
read_memory_fn=read_memory_fn,

Tools/WebServer/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
from fpb_inject import serial_open
3838
from services.device_worker import start_worker
3939
from services.file_watcher_manager import restore_file_watcher
40-
from utils.net import is_port_available as check_port_available
40+
from utils.net import is_port_available as check_port_available # noqa: same API
4141
from utils.net import get_port_owner
4242
from utils.port_lock import PortLock
4343

Tools/WebServer/tests/test_utils_net.py

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class TestIsPortAvailable(unittest.TestCase):
2727
def test_available_port(self):
2828
"""An unused port should be available."""
2929
# Use a random high port unlikely to be in use
30-
self.assertTrue(is_port_available(59123))
30+
self.assertTrue(is_port_available("127.0.0.1", 59123))
3131

3232
def test_occupied_port(self):
3333
"""A port with a listener should not be available."""
@@ -37,7 +37,7 @@ def test_occupied_port(self):
3737
s.listen(1)
3838
port = s.getsockname()[1]
3939
try:
40-
self.assertFalse(is_port_available(port))
40+
self.assertFalse(is_port_available("127.0.0.1", port))
4141
finally:
4242
s.close()
4343

@@ -49,7 +49,7 @@ def test_port_after_close(self):
4949
s.listen(1)
5050
port = s.getsockname()[1]
5151
s.close()
52-
self.assertTrue(is_port_available(port))
52+
self.assertTrue(is_port_available("127.0.0.1", port))
5353

5454

5555
class TestGetPortOwner(unittest.TestCase):
@@ -130,33 +130,32 @@ class TestCheckAndFreePort(unittest.TestCase):
130130

131131
def test_available_port(self):
132132
"""Should return True immediately for a free port."""
133-
self.assertTrue(check_and_free_port(59126))
133+
self.assertTrue(check_and_free_port("127.0.0.1", 59126))
134134

135135
@patch("utils.net.kill_port_owner", return_value=True)
136136
@patch("utils.net.is_port_available", return_value=False)
137137
def test_occupied_then_freed(self, mock_avail, mock_kill):
138138
"""Should try to kill and return True on success."""
139-
self.assertTrue(check_and_free_port(12345))
139+
self.assertTrue(check_and_free_port("127.0.0.1", 12345))
140140
mock_kill.assert_called_once_with(12345)
141141

142142
@patch("utils.net.kill_port_owner", return_value=False)
143143
@patch("utils.net.is_port_available", return_value=False)
144144
def test_occupied_kill_fails(self, mock_avail, mock_kill):
145145
"""Should return False when kill fails."""
146-
self.assertFalse(check_and_free_port(12345))
146+
self.assertFalse(check_and_free_port("127.0.0.1", 12345))
147147

148148

149149
class TestGDBPortConflict(unittest.TestCase):
150150
"""Integration test: GDB server port conflict detection."""
151151

152152
@patch("utils.net.get_port_owner")
153-
def test_gdb_manager_uses_check_and_free_port(self, mock_owner):
154-
"""start_external_gdb_server should call check_and_free_port."""
155-
153+
def test_gdb_manager_checks_port(self, mock_owner):
154+
"""start_external_gdb_server should check port availability."""
156155
mock_owner.return_value = None
157156

158157
with patch(
159-
"core.gdb_manager.check_and_free_port", return_value=True
158+
"core.gdb_manager.is_port_available", return_value=True
160159
) as mock_check:
161160
with patch("core.gdb_manager.GDBRSPBridge") as mock_bridge_cls:
162161
mock_bridge = MagicMock()
@@ -174,19 +173,53 @@ def test_gdb_manager_uses_check_and_free_port(self, mock_owner):
174173

175174
result = start_external_gdb_server(state)
176175
self.assertTrue(result)
177-
mock_check.assert_called_once_with(3333)
176+
mock_check.assert_called_once_with("127.0.0.1", 3333)
178177

179178
def test_gdb_manager_rejects_occupied_port(self):
180-
"""start_external_gdb_server should fail if port can't be freed."""
181-
with patch("core.gdb_manager.check_and_free_port", return_value=False):
182-
from core.gdb_manager import start_external_gdb_server
183-
184-
state = MagicMock()
185-
state.device.external_gdb_port = 3333
186-
state.external_gdb_bridge = None
187-
188-
result = start_external_gdb_server(state)
189-
self.assertFalse(result)
179+
"""start_external_gdb_server should fail with detailed info if port can't be freed."""
180+
with patch("core.gdb_manager.is_port_available", return_value=False):
181+
with patch(
182+
"core.gdb_manager.get_port_owner",
183+
return_value={
184+
"pid": 999,
185+
"name": "python",
186+
"cmdline": "python main.py",
187+
},
188+
):
189+
with patch("core.gdb_manager.kill_port_owner", return_value=False):
190+
from core.gdb_manager import start_external_gdb_server
191+
192+
state = MagicMock()
193+
state.device.external_gdb_port = 3333
194+
state.external_gdb_bridge = None
195+
196+
result = start_external_gdb_server(state)
197+
self.assertFalse(result)
198+
199+
def test_gdb_manager_kills_stale_and_starts(self):
200+
"""start_external_gdb_server should kill stale process and succeed."""
201+
with patch("core.gdb_manager.is_port_available", return_value=False):
202+
with patch(
203+
"core.gdb_manager.get_port_owner",
204+
return_value={"pid": 888, "name": "python", "cmdline": "old"},
205+
):
206+
with patch("core.gdb_manager.kill_port_owner", return_value=True):
207+
with patch("core.gdb_manager.GDBRSPBridge") as mock_bridge_cls:
208+
mock_bridge = MagicMock()
209+
mock_bridge.start.return_value = 3333
210+
mock_bridge.is_running = False
211+
mock_bridge_cls.return_value = mock_bridge
212+
213+
from core.gdb_manager import start_external_gdb_server
214+
215+
state = MagicMock()
216+
state.device.external_gdb_port = 3333
217+
state.device.elf_path = None
218+
state.device.download_chunk_size = 1024
219+
state.external_gdb_bridge = None
220+
221+
result = start_external_gdb_server(state)
222+
self.assertTrue(result)
190223

191224

192225
if __name__ == "__main__":

Tools/WebServer/utils/net.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
logger = logging.getLogger(__name__)
1818

1919

20-
def is_port_available(port, host="127.0.0.1"):
20+
def is_port_available(host, port):
2121
"""Check if a TCP port is available by attempting to connect.
2222
2323
Returns True if the port is free, False if something is listening.
@@ -136,7 +136,7 @@ def kill_port_owner(port, timeout=1.0):
136136
return False
137137

138138

139-
def check_and_free_port(port, host="127.0.0.1"):
139+
def check_and_free_port(host, port):
140140
"""Check if a TCP port is available; if not, kill the occupying process.
141141
142142
Returns True if the port is available (or was freed), False if still occupied.

0 commit comments

Comments
 (0)