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

Fix port allocation race condition during tests. #215

Merged
merged 5 commits into from Apr 17, 2019
Merged

Fix port allocation race condition during tests. #215

merged 5 commits into from Apr 17, 2019

Conversation

jeremyje
Copy link
Contributor

@jeremyje jeremyje commented Apr 16, 2019

This is done by now keeping the port open while calling next port library. This prevents races were 2 processes think the same port is available during the old model which was check open, close, blind reopen.

This change replaces nextport with using :0 as the random port number as well as providing a class to get the port only once to prevent multiple threads from using it.

Also increasing the test runs to 25x from 10x to reduce the risk of introducing more flakes.

Resolves #205.
Resolves #213

@yfei1
Copy link
Collaborator

yfei1 commented Apr 17, 2019

I found this article might help to resolve this issue with cleaner codes.

@jeremyje
Copy link
Contributor Author

That's useful for obsolete nextport but the plumbing parts still are relevant.

internal/port/port.go Outdated Show resolved Hide resolved
@jeremyje
Copy link
Contributor Author

I've removed nextport since :0 does the same thing. It's now just reduced to single handoff port listener.

@jeremyje jeremyje merged commit f464b0b into googleforgames:master Apr 17, 2019
@jeremyje jeremyje deleted the promrace branch April 17, 2019 18:55
@jeremyje jeremyje added this to the v0.5.0 milestone Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider disabling -race (tsan) flag from Open Match tests. Race Condition in Frontend tests.
3 participants