Conversation
Native backend ports were reserved by binding port 0, which the OS assigns from its ephemeral range; an unrelated outgoing connection can take that same port as its source port while it sits in TIME_WAIT, causing Bandit/Erlang to fail with eaddrinuse on reopen. Move backend reservation into Ports.ts so it shares the below-ephemeral scan with public auto allocation: it skips ports claimed by any saved stack and, reusing the same accepts-based occupancy check acquire uses for explicit public ports, ports held by a live listener on loopback or wildcard (a loopback-only bind can silently coexist with an existing wildcard listener on macOS, BSD and Windows, masking the real occupant). It never persists the backend port as a claim of its own. A launch attempt that loses its bind to another listener now excludes that port from the next retry's scan, so retries advance instead of repeating the same losing candidate. The release-before-child-bind window and the retry count are unchanged.
Contributor
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews were available: Claude reported four findings; Codex reported none. Code inspection confirms a cross-stack port-overlap bug, serial probe latency when connections reach the timeout, and repeated claims-file reads. The restart/TIME_WAIT concern remains uncertain because the native artifacts' socket options could not be verified. Runtime tests were not run; checkout dependencies are absent.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | packages/stack/src/Ports.ts:155 |
port-allocation |
claude | Native backend ports now share the public auto-allocation range without being persisted as claims. Public auto-allocation does not check occupancy, so on platforms permitting overlapping loopback and wildcard binds, a container-runtime stack can save a public port already serving another stack's native backend. Loopback traffic then reaches that backend instead of the intended public proxy. |
| 🟡 MINOR | packages/stack/src/Ports.ts:155 |
port-allocation |
claude | Deterministic backend selection retries the same initial candidate after restart or reopen. If a native service binds without SO_REUSEADDR and leaves server-side TIME_WAIT sockets, the reservation probe could accept a port that the child cannot bind, consuming collision retries. |
| 🟡 MINOR | packages/stack/src/Ports.ts:161 |
performance |
claude | Every native backend candidate now awaits an occupancy probe before binding. When refused loopback connections exceed the 250 ms timeout, as documented for Windows, each vacant candidate adds approximately 250 ms. Sequential endpoint reservations accumulate this delay on every launch attempt. |
| ⚪ NIT | packages/stack/src/Ports.ts:149 |
performance |
claude | Each backend endpoint reservation re-executes state.claims, repeatedly reading and decoding all saved stack claim documents within one service's reservation batch. |
Stats
Claude findings: 4 · Codex findings: 0 · Confirmed: 3 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Review on the backend-port-reservation fix found a regression and two robustness gaps it introduced: - Public auto allocation (Ports.ts acquire) bound fresh candidates without checking occupancy first. On macOS, BSD and Windows a container stack's wildcard public bind can succeed on a port where another stack's native backend already listens on loopback, so loopback traffic reached the backend instead of the proxy. Auto allocation now reuses the same accepts-based occupancy check (loopbackOccupied) backend reservation uses, so one mechanism owns whether a managed port is free for both allocators. - Backend port reservation scanned from a start derived from hash(stackId:key). Backend ports aren't persisted, so stability bought nothing, and a reopened stack retried its previous incarnation's port, which can still be in server-side TIME_WAIT: the Node probe sets SO_REUSEADDR and passes, while a child process without it fails, consuming a retry. The start is now drawn from the injected Crypto service per reservation; claim and exclusion skipping are unchanged. - A reservation batch re-read state.claims once per endpoint instead of once per attempt. reserveEndpoints now reads it once and shares the snapshot across the batch; a later retry attempt still re-reads it. - A service's endpoints now reserve concurrently instead of serially, since each reservation holds its bound probe listener until the batch releases, so two endpoints can't settle on the same port.
This branch has not been deployed
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.
On the native runtime, a reopened stack intermittently failed to start Logflare with
port 46477 already in use/eaddrinuse. This showed up in the native "eager lifecycle and reopen" stack E2E on Linux CI.ProcessRecipereserved each native backend port by binding port 0 and releasing it before spawning the service. That port comes from the OS ephemeral range (Linux: 32768–60999). In the gap, an unrelated outgoing connection can take it as its source port and leave it in TIME_WAIT, and the service's own bind then fails.Backend ports are now reserved by
Ports.ts, with the same below-ephemeral scan that public auto allocation uses (ADR 0017):accepts-based check as public ports. A loopback-only probe bind can coexist with a wildcard listener on macOS, BSD and Windows, which hid ports already published by Docker.The window between releasing the probe and the child binding, and the retry count, are unchanged.