Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reserve HTTP server port #215

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Reserve HTTP server port #215

merged 2 commits into from
Aug 12, 2024

Conversation

pNre
Copy link
Contributor

@pNre pNre commented Aug 9, 2024

Ciao,
we've been relying on SBTUITestTunnel to UI tests a fairly large codebase with 2k+ UI tests.
When running tests in parallel on multiple simulators (4+) with IPC disabled, we've been encountering failures due to UITestTunnelServer being unable to bind to the HTTP server address:

[UITestTunnelServer] Failed to start server on port 61692. Error Domain=NSPOSIXErrorDomain Code=48 "Address already in use" UserInfo={NSLocalizedDescription=Address already in use}

Through debugging I've identified a potential race in [SBTUITestTunnelClient findOpenPort] which can lead to the system allocating the same port in parallel processes, leading to the aforementioned error.

In this PR I've refactored findOpenPort to temporarily "reserve" the port which should prevent the system from allocating it to other processes for a certain time frame (hopefully long enough to start the server and bind the address)

@tcamin
Copy link
Member

tcamin commented Aug 9, 2024

Ciao! Could you elaborate, possibly expanding the code comment you added, to explain why this is actually working? I may be wrong but it seems that the trick of "reserving" the port by opening/closing a socket temporarily works because SO_REUSEADDR is set here and the system won't reuse the port while there are connection in TIME_WAIT state. If this is the case a more in-depth explanation would greatly help to understand that part of the code.

Thanks!

@pNre
Copy link
Contributor Author

pNre commented Aug 9, 2024

Yes, that's the idea. I've amended the comment to provide a more detailed explanation.

Copy link
Member

@tcamin tcamin left a comment

Choose a reason for hiding this comment

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

Great fix, thanks!

@tcamin tcamin merged commit e4ece17 into Subito-it:master Aug 12, 2024
1 check passed
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