Skip to content

check port with bind over connect_ex#5564

Merged
adhami3310 merged 4 commits into
mainfrom
check-port-with-bind-over-connect_ex
Jul 11, 2025
Merged

check port with bind over connect_ex#5564
adhami3310 merged 4 commits into
mainfrom
check-port-with-bind-over-connect_ex

Conversation

@adhami3310
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Improves port availability checking in reflex/utils/processes.py by switching from socket.connect_ex() to socket.bind() for more reliable port verification.

  • Changed port checking method in reflex/utils/processes.py from connect_ex to bind for more accurate availability detection
  • Added PermissionError handling to handle cases where port binding requires elevated privileges
  • Renamed _is_address_responsive to _can_bind_at_port to better reflect the new implementation's purpose
  • Inverted return logic in is_process_on_port to align with bind-based checking semantics

1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jul 11, 2025

CodSpeed Performance Report

Merging #5564 will not alter performance

Comparing check-port-with-bind-over-connect_ex (c39da8c) with main (3c687f4)

Summary

✅ 8 untouched benchmarks

@adhami3310 adhami3310 merged commit 5890982 into main Jul 11, 2025
38 of 41 checks passed
@adhami3310 adhami3310 deleted the check-port-with-bind-over-connect_ex branch July 11, 2025 20:21
@sorousch9
Copy link
Copy Markdown

sorousch9 commented Jul 15, 2025

@adhami3310 In Windows Subsystem for Linux WSL, the recursive change_port call can overflow the call stack and trigger a RecursionError.

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.

3 participants