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 race condition in ReplayGain test #4345

Merged
merged 1 commit into from
Oct 2, 2021

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Oct 1, 2021

This test caused a race because it called slotLoadTrack instead of loadTrack, so it did not wait for the track to actually load. The test was quitting before the caching reader had a chance to finish setting up, so when the test was getting torn down there was a chance that the reader would try to access the objects while they were being destructed.

Because the test infrastructure has its own destruction code, it's not clear if this crash could occur in regular code.

Fixes https://bugs.launchpad.net/mixxx/+bug/1940589

@daschuer
Copy link
Member

daschuer commented Oct 1, 2021

Thank you very much. Can you link here the original report for later reference?
Do you know exactly where the segfault has happened? Are we sure all safety fences are installed that this never happens in the productive code?

@Be-ing
Copy link
Contributor

Be-ing commented Oct 1, 2021

Thanks for fixing this. It's not clear to me from looking at the diff what was causing the problem.

Can you link here the original report for later reference?

Yes, please put full URLs to bug reports in commit messages.

@ywwg ywwg force-pushed the replaygain-test-fix branch 2 times, most recently from 955be72 to b872f68 Compare October 1, 2021 21:30
This test caused a race because it called slotLoadTrack instead of loadTrack, so it did not wait for the track to actually load. The test was quitting before the caching reader had a chance to finish setting up, so when the test was getting torn down there was a chance that the reader would try to access the objects while they were being destructed.

Because the test infrastructure has its own destruction code, it's not clear if this crash could occur in regular code.

Fixes https://bugs.launchpad.net/mixxx/+bug/1940589
Copy link
Contributor

@Be-ing Be-ing left a comment

Choose a reason for hiding this comment

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

waiting for CI

@ywwg
Copy link
Member Author

ywwg commented Oct 2, 2021

looks good

@Holzhaus Holzhaus merged commit 8dfe58b into mixxxdj:main Oct 2, 2021
@uklotzde
Copy link
Contributor

uklotzde commented Oct 6, 2021

That didn't work out: https://github.com/mixxxdj/mixxx/runs/3808718927

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants