fix(serve): fail fast on occupied port before preload#1526
Merged
Conversation
Contributor
Author
|
Opened #1526 with a fix for this. |
Owner
|
Thanks for the clean fix. You nailed the root cause here: uvicorn runs the ASGI lifespan (so our pinned-model preload) before it binds the port, so an occupied port burned a whole load/unload cycle. Pre-binding via |
This was referenced Jun 2, 2026
fqx
added a commit
to fqx/omlx
that referenced
this pull request
Jun 8, 2026
Fixes the regression introduced when jundot#1526 switched from uvicorn.run() to uvicorn.Config + bind_socket() + Server.run(sockets=[...]): the old approach silently accepted a list of hosts via asyncio.create_server(), but bind_socket() calls sock.bind((host, port)) which requires a string. Changes: - settings.py: normalise legacy YAML list values (host: [a, b]) to a comma-separated string on load so the web UI and server both see a str - cli.py (serve_command): parse comma-separated hosts, bind one socket per address before model preload (preserving the fail-fast guarantee from jundot#1526), then pass all sockets to Server.run() - cli.py (launch_command): pick the first bind address as the connect host, normalising wildcards (0.0.0.0, ::) to 127.0.0.1 - utils/network.py: add is_valid_bind_host() which accepts 0.0.0.0 and :: unlike is_valid_alias() which rejects unspecified addresses - admin/routes.py: validate each comma-separated part on save, returning a readable 400 before an invalid value can crash the server on restart - admin/static/js/dashboard.js: extract .msg from Pydantic error objects instead of joining raw objects (fixes [object Object] display), two sites - admin/i18n/{en,zh,ja,ko,zh-TW}.json: add host_multi_hint string and drop "(caution)" from host_custom label now that multi-address is valid - admin/templates/dashboard/_settings.html: show host_multi_hint hint below the custom host input when the custom mode is active Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jundot
pushed a commit
that referenced
this pull request
Jun 8, 2026
* fix: support comma-separated bind addresses in host setting Fixes the regression introduced when #1526 switched from uvicorn.run() to uvicorn.Config + bind_socket() + Server.run(sockets=[...]): the old approach silently accepted a list of hosts via asyncio.create_server(), but bind_socket() calls sock.bind((host, port)) which requires a string. Changes: - settings.py: normalise legacy YAML list values (host: [a, b]) to a comma-separated string on load so the web UI and server both see a str - cli.py (serve_command): parse comma-separated hosts, bind one socket per address before model preload (preserving the fail-fast guarantee from #1526), then pass all sockets to Server.run() - cli.py (launch_command): pick the first bind address as the connect host, normalising wildcards (0.0.0.0, ::) to 127.0.0.1 - utils/network.py: add is_valid_bind_host() which accepts 0.0.0.0 and :: unlike is_valid_alias() which rejects unspecified addresses - admin/routes.py: validate each comma-separated part on save, returning a readable 400 before an invalid value can crash the server on restart - admin/static/js/dashboard.js: extract .msg from Pydantic error objects instead of joining raw objects (fixes [object Object] display), two sites - admin/i18n/{en,zh,ja,ko,zh-TW}.json: add host_multi_hint string and drop "(caution)" from host_custom label now that multi-address is valid - admin/templates/dashboard/_settings.html: show host_multi_hint hint below the custom host input when the custom mode is active Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * revert: restore "(caution)" label on host_custom in all languages The caution warning is still valid — manually entering a bind address can inadvertently expose the server, and supporting multiple addresses makes that risk more rather than less relevant. * fix: put host_multi_hint inside right-side flex column, below the input * fix: reject IP-shaped invalid hosts and handle comma-separated bind hosts is_valid_bind_host(): add a digit-dot regex guard so strings like 999.999.999.999 that fail ipaddress.ip_address() are not silently accepted as hostnames (digit-only dotted labels satisfy RFC 1123). detect_server_aliases(): split the host parameter on commas before checking for loopback/wildcard addresses, so values like "127.0.0.1, ::1" no longer drop localhost from the alias list. Adds focused tests covering valid IPv4/IPv6 (including unspecified addresses and IPv4-mapped IPv6), valid hostnames, IP-shaped-but-invalid values of various arity, malformed hostnames, and wrong types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: tighten hostname validation — reject all-digit last labels in dotted names is_valid_hostname(): require the rightmost label of any dotted name to contain at least one letter. IANA has never delegated a numeric TLD, so an all-digit last label (e.g. "999" in "999.999.999.999") indicates an IP-shaped string, not a real hostname. This mirrors the approach used by the validators PyPI library (python-validators), whose domain regex ends with [a-z] to enforce the same constraint. Single-label names (no dots) are local hostnames and are exempt from the TLD constraint. The IP-shaped fast-path guard in is_valid_bind_host() is retained as a cheap short-circuit, but the root fix is now in is_valid_hostname() so both is_valid_bind_host() and is_valid_alias() benefit. Adds is_valid_hostname tests for the new behaviour, and fixes test_accepts_hostname_with_dashes_instead_of_dots to verify that single-label names without letters (e.g. "192-168-1-1") are still accepted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1524.
Summary
uvicorn.Server(...).run(sockets=[...])Problem
omlx servepreloads pinned models during FastAPI lifespan startup. Uvicorn's normal startup path runs lifespan before binding the host/port, so a second process on the same port can still load and unload pinned models before exiting withEADDRINUSE.Under a restart supervisor, that can turn a simple port conflict into repeated expensive model load/unload cycles.
Approach
Use
uvicorn.Config.bind_socket()before importingomlx.serveror callinginit_server(...). If the bind fails, the process exits before server setup. If it succeeds, the existing setup path runs and uvicorn receives the already-bound socket.This keeps the change scoped to the
omlx serveCLI path.Tests
uv run pytest tests/test_cli.py -quv run python -m compileall -q omlx/cli.py tests/test_cli.pygit diff --check