Skip to content

fix: detect port conflicts on loopback addresses, not just 0.0.0.0#345

Merged
jdx merged 1 commit into
jdx:mainfrom
gaojunran:fix-port-detect
Apr 11, 2026
Merged

fix: detect port conflicts on loopback addresses, not just 0.0.0.0#345
jdx merged 1 commit into
jdx:mainfrom
gaojunran:fix-port-detect

Conversation

@gaojunran

Copy link
Copy Markdown
Contributor

On macOS/BSD, Rust's TcpListener::bind sets SO_REUSEADDR by default, which allows binding 0.0.0.0:port to succeed even when 127.0.0.1:port (or ::1:port) is already occupied — because the kernel considers them different addresses. This caused check_ports_available() to report a port as available when it was actually in use on loopback, leading to daemons silently falling back to a different port while pitchfork still reported the original one.

Now check 0.0.0.0, 127.0.0.1, and ::1 for each candidate port so that a conflict on any loopback interface is detected reliably.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the port availability check in src/supervisor/lifecycle.rs to verify multiple addresses (0.0.0.0, 127.0.0.1, and ::1), addressing false negatives on platforms like macOS/BSD where SO_REUSEADDR is the default. Feedback was provided regarding the IPv6 check, which may incorrectly report a port conflict on systems where IPv6 is disabled; handling specific IO errors for the ::1 address was suggested to ensure compatibility across different environments.

Comment thread src/supervisor/lifecycle.rs
@greptile-apps

greptile-apps Bot commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a false-negative in check_ports_available on macOS/BSD: because Rust sets SO_REUSEADDR by default, binding 0.0.0.0:port could succeed even when 127.0.0.1:port was already occupied, so the port appeared free. The fix probes all three addresses (0.0.0.0, 127.0.0.1, ::1) and only treats AddrInUse as a true conflict — other errors (e.g. EADDRNOTAVAIL on IPv6-disabled systems) are silently skipped, which also resolves the previously raised concern about IPv6-less environments.

Confidence Score: 5/5

Safe to merge — the fix is correct, well-scoped, and the previously raised P1 concern is already addressed in the current code.

No P0 or P1 findings remain. The prior thread's concern about Err(_) silently swallowing EADDRNOTAVAIL on IPv6-disabled hosts is resolved: the code now only treats AddrInUse as a conflict and skips all other errors. The three-address probe correctly covers the macOS/BSD SO_REUSEADDR edge case.

No files require special attention.

Important Files Changed

Filename Overview
src/supervisor/lifecycle.rs Expands check_ports_available to probe 0.0.0.0, 127.0.0.1, and ::1 and correctly treats non-AddrInUse errors (e.g. EADDRNOTAVAIL on IPv6-disabled hosts) as non-conflicts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[check_ports_available called] --> B[For each candidate port]
    B --> C{port == 0?}
    C -- yes --> B
    C -- no --> D[spawn_blocking]
    D --> E[Try bind 0.0.0.0:port]
    E -- Ok --> F[drop listener]
    F --> G[Try bind 127.0.0.1:port]
    G -- Ok --> H[drop listener]
    H --> I[Try bind ::1:port]
    I -- Ok --> J[drop listener → return true]
    E -- AddrInUse --> K[return false]
    G -- AddrInUse --> K
    I -- AddrInUse --> K
    E -- Other error --> G
    G -- Other error --> I
    I -- Other error --> J
    K --> L[port_check = false]
    J --> M[port_check = true]
    L --> N[all_available = false, break]
    M --> O{all ports available?}
    O -- yes --> P[return resolved ports]
    O -- no --> Q{auto_bump?}
    Q -- yes --> R[increment bump_offset, retry]
    Q -- no --> S[return PortError]
Loading

Reviews (2): Last reviewed commit: "fix: detect port conflicts on loopback a..." | Re-trigger Greptile

Comment thread src/supervisor/lifecycle.rs
On macOS/BSD, Rust's TcpListener::bind sets SO_REUSEADDR by default,
which allows binding 0.0.0.0:port to succeed even when 127.0.0.1:port
(or ::1:port) is already occupied — because the kernel considers them
different addresses. This caused check_ports_available() to report a
port as available when it was actually in use on loopback, leading to
daemons silently falling back to a different port while pitchfork
still reported the original one.

Now check 0.0.0.0, 127.0.0.1, and ::1 for each candidate port so that
a conflict on any loopback interface is detected reliably.
@jdx jdx merged commit a657e0a into jdx:main Apr 11, 2026
5 checks passed
@jdx jdx mentioned this pull request Apr 11, 2026
jdx added a commit that referenced this pull request Apr 12, 2026
## 🤖 New release

* `pitchfork-cli`: 2.5.0 -> 2.6.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [2.6.0](v2.5.0...v2.6.0) -
2026-04-12

### Added

- *(proxy)* auto start when visiting the proxied URL
([#347](#347))

### Fixed

- some issues related to sudo supervisor
([#323](#323))
- *(port)* should fail when ready_port is in use
([#350](#350))
- *(deps)* update rcgen to 0.14
([#349](#349))
- *(deps)* update reqwest to 0.13
([#348](#348))
- detect port conflicts on loopback addresses, not just 0.0.0.0
([#345](#345))
- narrow REAPED_STATUSES cfg to non-Linux unix only
([#346](#346))
- *(deps)* update rust crate ratatui to 0.30
([#331](#331))
- *(deps)* update rust crate toml to v1
([#344](#344))
- *(deps)* update rust crate strum to 0.28
([#334](#334))
- *(deps)* update rust crate notify-debouncer-full to 0.7
([#330](#330))
- *(deps)* update rust crate nix to 0.31
([#329](#329))
- *(deps)* update rust crate listeners to 0.5
([#328](#328))
- *(deps)* update rust crate sysinfo to 0.38
([#335](#335))
- *(deps)* update rust crate cron to 0.16
([#324](#324))
- *(deps)* update rust crate crossterm to 0.29
([#325](#325))

### Other

- *(deps)* update rust crate rmcp to v1.4.0
([#327](#327))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Low risk: this PR only bumps the crate version and updates release
notes, with no runtime code changes.
> 
> **Overview**
> Prepares the `pitchfork-cli` **v2.6.0** release by bumping the package
version from `2.5.0` to `2.6.0` in `Cargo.toml`/`Cargo.lock`.
> 
> Updates `CHANGELOG.md` with the `2.6.0` release notes (proxy
auto-start behavior, several fixes, and dependency updates).
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
faea6c5. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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.

2 participants