[fix](cli): detect bound ports before launch#2071
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the port checker utility to determine port availability by attempting to bind to the port, and adds a corresponding unit test. The reviewer noted that on non-Windows platforms, attempting to bind to a port in the TIME_WAIT state without setting SO_REUSEADDR will fail, leading to false negatives. They provided a code suggestion to conditionally set SO_REUSEADDR on non-Windows platforms to avoid this issue while preventing false positives on Windows.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| bind_host = "" if host == "0.0.0.0" else host | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | ||
| sock.bind((bind_host, port)) | ||
| return True |
There was a problem hiding this comment.
On non-Windows platforms (like Linux and macOS), if a port is in the TIME_WAIT state (for example, right after a server process is stopped), attempting to bind to it without setting the SO_REUSEADDR socket option will fail with an OSError (Address already in use). Since standard server implementations typically set SO_REUSEADDR to allow binding to ports in TIME_WAIT, this port checker will incorrectly report such ports as unavailable (false negatives).
To prevent this, we should set SO_REUSEADDR on non-Windows platforms. We must avoid setting it on Windows, because on Windows, SO_REUSEADDR allows multiple sockets to bind to the exact same port even if one is actively listening, which would cause false positives (reporting an active port as available).
| bind_host = "" if host == "0.0.0.0" else host | |
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | |
| sock.bind((bind_host, port)) | |
| return True | |
| import sys | |
| bind_host = "" if host == "0.0.0.0" else host | |
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | |
| if sys.platform != "win32": | |
| sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) | |
| sock.bind((bind_host, port)) | |
| return True |
There was a problem hiding this comment.
Thanks, this is a good catch. I updated the helper to set SO_REUSEADDR only when sys.platform != win32, so Linux/macOS bind checks better match normal server reuse behavior while avoiding Windows false positives. I also added a focused test that mocks the non-Windows branch and verifies setsockopt(SO_REUSEADDR) is applied before bind().
Validation rerun:
mamba run -n prw-ktransformers-py311 python -m unittest per_commit.test_port_checker -v- passed, 2 tests- local bound-but-not-listening behavior proof - passed
git diff --check- passed
|
Yes, the problem is real. The old |
* [fix](cli): detect bound ports before launch * [fix](cli): align port reuse check by platform
* [fix](cli): detect bound ports before launch * [fix](cli): align port reuse check by platform
* [fix](cli): detect bound ports before launch * [fix](cli): align port reuse check by platform
What does this PR do?
Fixes the kt-kernel CLI port availability check so it tests whether the server can actually bind the requested host/port, instead of probing whether a client can connect to it.
Fixes #2072
What Problem This Solves
kt runuses the interactive setup flow to choose and persist server launch settings. The host/port validation happens before the configuration is saved or reused, so the helper needs to answer the same question the later server launch will ask: can this process bind the requested local address?The affected path is:
There are two user-visible entry points into the same helper:
Before this change,
is_port_available()usedsocket.connect_ex()to decide whether the port was available. That checks client reachability: whether a client can connect to a server that is already listening at the target address. The CLI needs server bindability: whether the launch process can reserve that local host/port withbind().Those two checks differ in an important edge case. Another process can already hold a port by calling
bind()without callinglisten()yet. In that state:That means the CLI can print that a port is available, save or suggest that port, and only fail later when the actual server launch tries to bind it. The validation step is therefore checking the wrong side of the socket lifecycle.
Change
connect_ex()reachability probe with a short-livedbind()check.0.0.0.0handling by binding through the wildcard host.SO_REUSEADDRonly on non-Windows platforms so Linux/macOS checks match typical server reuse behavior without introducing Windows false positives.SO_REUSEADDRbranch.Evidence
The regression test covers the exact false-positive shape:
The reviewer-requested platform branch is also covered with a mocked Linux platform check:
Post-fix local behavior proof:
Possible call chain / impact
Affected interactive paths:
This PR only changes the CLI helper used to decide whether a TCP server port can be bound. It does not change model loading, SGLang launch arguments, network serving behavior after a port is selected, or non-CLI kernel code.
Validation
mamba run -n prw-ktransformers-py311 python -m unittest per_commit.test_port_checker -vfromkt-kernel/test- passed, 2 testsgit diff --check- passedI also attempted
mamba run -n prw-ktransformers-py311 python run_suite.py --hw cpu --suite defaultfromkt-kernel/test; on this Windows environment it stops before the suite because the runner invokespython3, which is not available here (test_basic_cpu.pyexits 9009). The focused regression tests above run successfully with the same Python 3.11 conda environment.Before submitting