Skip to content

[Tests] Make Mutex.cpp tests more reliable. #23816

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

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

mikeash
Copy link
Contributor

@mikeash mikeash commented Apr 5, 2019

Many of these tests would fail if one of the test threads didn't begin execution within 100 milliseconds. They would hit while (!done) the very first time and never execute the loop body. That does happen from time to time, and the result would be a spurious failure. Change the loops to do {} while(!done) to ensure they always execute at least one iteration.

Many tests also had the test threads concurrently write results to a std::vector, which appeared to be causing some failures in my local testing when I had extra sleeps inserted to simulate pathological scheduling. Change the results vectors to be vectors of std::atomic to ensure that this concurrent writing is safe.

These changes shouldn't affect its ability to test the functionality it intends to test.

rdar://problem/49386389

@mikeash mikeash requested a review from harlanhaskins April 5, 2019 17:33
@mikeash
Copy link
Contributor Author

mikeash commented Apr 5, 2019

@swift-ci please test

Copy link
Contributor

@harlanhaskins harlanhaskins left a comment

Choose a reason for hiding this comment

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

Thanks! My only request is that you add a comment somewhere about why we can't just use a while in future additions.

@mikeash
Copy link
Contributor Author

mikeash commented Apr 5, 2019

Good idea, I'll do that.

Many of these tests would fail if one of the test threads didn't begin execution within 100 milliseconds. They would hit while (!done) the very first time and never execute the loop body. That does happen from time to time, and the result would be a spurious failure. Change the loops to do {} while(!done) to ensure they always execute at least one iteration.

Many tests also had the test threads concurrently write results to a std::vector, which appeared to be causing some failures in my local testing when I had extra sleeps inserted to simulate pathological scheduling. Change the results vectors to be vectors of std::atomic to ensure that this concurrent writing is safe.

These changes shouldn't affect its ability to test the functionality it intends to test.

rdar://problem/49386389
@mikeash mikeash force-pushed the improve-mutex-tests branch from 9b1364e to 6172f72 Compare April 5, 2019 20:40
@mikeash
Copy link
Contributor Author

mikeash commented Apr 5, 2019

@swift-ci please smoke test and merge

1 similar comment
@mikeash
Copy link
Contributor Author

mikeash commented Apr 8, 2019

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 98200db into swiftlang:master Apr 8, 2019
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.

4 participants