Skip to content

feat(inbounds): make port allocation atomic with an opt-in reservation table - #6185

Open
n0ctal wants to merge 2 commits into
MHSanaei:mainfrom
n0ctal:upstream-port-reservations
Open

feat(inbounds): make port allocation atomic with an opt-in reservation table#6185
n0ctal wants to merge 2 commits into
MHSanaei:mainfrom
n0ctal:upstream-port-reservations

Conversation

@n0ctal

@n0ctal n0ctal commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Add an opt-in port reservation table that makes inbound port allocation atomic, behind XUI_ENFORCE_PORT_RESERVATIONS=1. Default behaviour is unchanged.

This is a proposal rather than a drop-in fix — it introduces a table, so I would rather agree the shape with you than land it quietly. Happy to reshape or split it.

Why

checkPortConflict is a read, and the write that claims the port happens afterwards in a separate step. Two requests that create or update inbounds concurrently can both pass the check while neither has claimed anything yet, and both then take the same port.

The panel ends up holding two inbounds on one port. Nothing in the panel objects — the conflict only surfaces when xray refuses to start, or when one listener wins and the other silently never accepts traffic. Reproducing it needs two overlapping requests, which is exactly what an API-driven or multi-operator setup produces, and it is invisible in a single-operator UI session.

Tightening the existing check cannot close this: any read-then-write pair has the same window. The claim has to be the same operation as the check.

Scope

  • New inbound_port_reservations table: (inbound_id, node_scope, listen, port, transport) with two unique indexes — one on the exact tuple, one on owner+transport — so a duplicate claim fails at the database rather than at review time.
  • reserveInboundPortsTx / deleteInboundPortReservationsTx run inside the same transaction as the inbound write.
  • Postgres advisory locks (lockPortReservationKeysTx) serialise concurrent claims on the same key; 0x5855 is a fixed namespace with node_scope and port packed into the remaining bits, so keys cannot collide.
  • An in-process mutex (lockPortReservationKeys) covers the same key within one panel.
  • AddInbound / UpdateInbound keep calling checkPortConflict unchanged when the gate is off.
  • PrepareInboundPortReservations() at startup creates the table and backfills existing inbounds.

Validation

  • go build ./internal/... clean.
  • go test ./internal/web/service/ green on main at ece16559 with the gate off, which is the default.
  • The feature's own tests set the gate per-case with t.Setenv and cover concurrent claims, transport separation and node scoping.

Risk

Off by default, so an install that does not set the variable behaves exactly as today.

One honest caveat: with the gate on, the table must exist, which means PrepareInboundPortReservations() must have run. That holds for a normal start, but a test or embedding that constructs the service without going through startup will fail with no such table: inbound_port_reservations rather than degrading. If you would prefer the gate to fall back to checkPortConflict when the table is absent, that is a small change and I am glad to make it.

The advisory-lock path is Postgres-only; SQLite relies on the unique indexes and the in-process mutex, which is sufficient for a single panel process but not for two processes on one SQLite file.

@n0ctal
n0ctal force-pushed the upstream-port-reservations branch from b5f0594 to 8d162f6 Compare August 8, 2026 02:15
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.

1 participant