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

Possible race condition in get_free_port #16754

Closed
georgeroman opened this issue May 9, 2019 · 4 comments · Fixed by #16819 or #16912
Closed

Possible race condition in get_free_port #16754

georgeroman opened this issue May 9, 2019 · 4 comments · Fixed by #16819 or #16912
Labels
infra priority:backlog wptrunner The automated test runner, commonly called through ./wpt run

Comments

@georgeroman
Copy link
Contributor

I am experimenting with WebDriver in Servo and came across with what might be a race condition in the get_free_port function. That is, sometimes when wptrunner runs the tests in parallel, two webdriver instances get assigned the same port.
You can find all the details here: servo/servo#22619 (comment).

@jdm jdm added webdriver wptrunner The automated test runner, commonly called through ./wpt run labels May 9, 2019
@jdm
Copy link
Contributor

jdm commented May 9, 2019

This may be a symptom of how Servo's integration with the webdriver server is different from other browsers. My understanding is that other browsers have a separate server that spawns instances of the browser, whereas Servo has the server integrated directly into each instance of the browser. If there are multiple threads starting multiple browsers, maybe we could use a mutex inside this code to avoid the race.

@gsnedders
Copy link
Member

I do wonder if every usage of get_free_port is racy in the face of parallel test execution?

This code has annoyed me for a long time, because it seems prone to raciness and there are multiple ways this could fail (heck, some other process could open that port!). It would definitely be nice to fix this more generally, though I don't really know how…

@gsnedders
Copy link
Member

From @georgeroman in #16819:

I think [every usage ] is [racy] since every usage of the function follows the same pattern of getting the first available port followed by saving the chosen port in excluded_ports.

So reopening for everywhere else.

@gsnedders
Copy link
Member

And for those wondering why we don't just use port "0" (i.e., let the system assign us a port): this is used when we need to pass an argument in (typically as a subprocess command-line argument) giving the port to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra priority:backlog wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants