-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(ai): verify OS port availability in assign_port to prevent cross-engine data-plane crosstalk (#1558) #1566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,7 @@ | |
|
|
||
| import time | ||
| import asyncio | ||
| import socket | ||
| import uuid | ||
| from typing import List | ||
| from fastapi import WebSocket | ||
|
|
@@ -695,22 +696,60 @@ def get_task(self, token: str) -> Task: | |
| # Extract and return the task instance | ||
| return control.task | ||
|
|
||
| @staticmethod | ||
| def _is_port_free(port: int) -> bool: | ||
| """ | ||
| Check whether a TCP port is actually free on the host. | ||
|
|
||
| The in-memory ``_allocated_ports`` list only knows about ports handed | ||
| out by *this* engine process. It cannot see ports held by another | ||
| engine (e.g. a second workspace, or an IDE engine alongside a CLI | ||
| engine) or any unrelated process. Two engines both defaulting to | ||
| ``base_port=20000`` would otherwise both hand out 20000/20001, and the | ||
| second engine's node host would silently dial the first engine's data | ||
| plane (see #1558). | ||
|
|
||
| Attempt a real bind on ``127.0.0.1:port`` to confirm availability. | ||
|
|
||
| Args: | ||
| port: TCP port number to probe. | ||
|
|
||
| Returns: | ||
| True if the port could be bound (free), False if it is in use. | ||
| """ | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | ||
| try: | ||
| # No SO_REUSEADDR: we want the bind to fail if another | ||
| # process (or engine) is already holding this port. | ||
| sock.bind(('127.0.0.1', port)) | ||
| return True | ||
| except OSError: | ||
| return False | ||
|
Comment on lines
+720
to
+727
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Map the relevant file and nearby symbols first.
ast-grep outline packages/ai/src/ai/modules/task/task_server.py --view expanded
# Read the relevant region around the port probe and allocator.
sed -n '700,780p' packages/ai/src/ai/modules/task/task_server.py
# Inspect the referenced test region.
sed -n '180,260p' packages/ai/src/ai/modules/task/test_task_server.py
# Search for other call sites or guards around port allocation.
rg -n "_is_port_free|assign_port|base_port|65535|OverflowError" packages/ai/src/ai/modules/taskRepository: rocketride-org/rocketride-server Length of output: 5692 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find the test file and inspect port-allocation tests.
fd -a "test_task_server.py" packages
rg -n "assign_port|_is_port_free|base_port|ephemeral|port" packages/ai/src/ai/modules/task -g '*test*'
# Read the likely test file if found.
test_file="$(fd -a "test_task_server.py" packages | head -n 1 || true)"
if [ -n "$test_file" ]; then
wc -l "$test_file"
sed -n '1,280p' "$test_file"
fi
# Probe socket behavior for out-of-range ports.
python3 - <<'PY'
import socket
for port in [65535, 65536, 70000, -1]:
try:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
s.bind(("127.0.0.1", port))
print(port, "OK")
except Exception as e:
print(port, type(e).__name__, e)
PYRepository: rocketride-org/rocketride-server Length of output: 243 🏁 Script executed: #!/bin/bash
set -euo pipefail
test_file="packages/ai/tests/ai/modules/task/test_task_server.py"
# Inspect the relevant test area.
wc -l "$test_file"
sed -n '1,280p' "$test_file"
# Confirm the socket exception type for out-of-range ports.
python3 - <<'PY'
import socket
for port in [65535, 65536, 70000, -1]:
try:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
s.bind(("127.0.0.1", port))
print(port, "OK")
except Exception as e:
print(port, type(e).__name__, str(e))
PYRepository: rocketride-org/rocketride-server Length of output: 10327 Guard 🤖 Prompt for AI Agents |
||
|
|
||
| def assign_port(self) -> int: | ||
| """ | ||
| Allocate available port from managed pool. | ||
|
|
||
| A candidate port must be free both in this engine's own | ||
| ``_allocated_ports`` list *and* on the host OS (verified with a real | ||
| bind), so two engines on the same machine never hand out the same | ||
| port and cross-connect their data planes (#1558). | ||
|
|
||
| Returns: | ||
| Available port number (base_port to base_port+9999 range) | ||
|
|
||
| Raises: | ||
| RuntimeError: If no ports available | ||
| """ | ||
| base_port = self._config.get('base_port', 20000) | ||
| # Search for available port | ||
| # Search for a port free both in our pool and on the host | ||
| for port in range(base_port, base_port + 10000): | ||
| if port not in self._allocated_ports: | ||
| self._allocated_ports.append(port) | ||
| return port | ||
| if port in self._allocated_ports: | ||
| continue | ||
| if not self._is_port_free(port): | ||
| continue | ||
| self._allocated_ports.append(port) | ||
| return port | ||
|
|
||
| raise RuntimeError(f'No available ports in the range {base_port}-{base_port + 9999}') | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: rocketride-org/rocketride-server
Length of output: 9074
🏁 Script executed:
Repository: rocketride-org/rocketride-server
Length of output: 586
🏁 Script executed:
Repository: rocketride-org/rocketride-server
Length of output: 6022
Hold the port reservation until the child binds
_is_port_free()only probes and closes the socket, soassign_port()can still hand out the same port to two concurrent engines before either child process binds it. Reserving the socket through startup or handing the fd to the listener is the safer fix.🤖 Prompt for AI Agents