Skip to content

fix(serve): fail fast on occupied port before preload#1526

Merged
jundot merged 1 commit into
jundot:mainfrom
azhangd:fix/fail-fast-port-conflict
May 30, 2026
Merged

fix(serve): fail fast on occupied port before preload#1526
jundot merged 1 commit into
jundot:mainfrom
azhangd:fix/fail-fast-port-conflict

Conversation

@azhangd

@azhangd azhangd commented May 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #1524.

Summary

  • bind the uvicorn serve socket before importing/initializing the server
  • fail occupied host/port startup before ASGI lifespan and pinned model preload
  • pass the pre-bound socket into uvicorn.Server(...).run(sockets=[...])
  • close the socket if server initialization fails before uvicorn takes ownership

Problem

omlx serve preloads 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 with EADDRINUSE.

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 importing omlx.server or calling init_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 serve CLI path.

Tests

  • uv run pytest tests/test_cli.py -q
  • uv run python -m compileall -q omlx/cli.py tests/test_cli.py
  • git diff --check

@azhangd

azhangd commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Opened #1526 with a fix for this.

@jundot

jundot commented May 30, 2026

Copy link
Copy Markdown
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 Config.bind_socket() and handing the socket to Server.run(sockets=...) is the right pattern, and the bind-before-import tests cover the ordering nicely. I'll merge this.

@jundot jundot merged commit b1d6880 into jundot:main May 30, 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

omlx serve preloads pinned models before failing on occupied port

2 participants