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 data race UB (audio thread pushing to its own message queue) triggered by WAV export #272

Merged
merged 1 commit into from
May 5, 2024

Conversation

nyanpasu64
Copy link
Collaborator

In a previous PR #137, I replaced audio thread communication with inter-thread message queues to prevent blocking the audio thread and interrupting playback. Unfortunately I missed a case where when you perform WAV export, the audio thread itself is pushing into the message queue intended for GUI-to-audio commands. This results in a UB data race on the lock-free ring buffer queue, which could result in missing or invalid commands being processed.

Separately I've noticed crashes upon WAV export where m_bRendering is true, but m_pWaveFile is null while calling m_pWaveFile->WriteWave(). I don't know how it happens, but it may be related to the above UB.

I'm fixing the data race by introducing a separate object for messages sent from the audio thread to itself. Hopefully it fixes the WAV export crash, but it's intermittent and I can't reproduce it on demand on the old code, nor prove that this change fixes it.

Changes in this PR:

  • Fix data race UB (audio thread pushing to its own message queue) triggered by WAV export

Hopefully fixes random crash upon WAV export.
@Gumball2415
Copy link
Collaborator

looks good to me

@nyanpasu64 nyanpasu64 merged commit 2f525d4 into main May 5, 2024
8 of 9 checks passed
@nyanpasu64 nyanpasu64 deleted the fix-wav-export-crash branch May 5, 2024 05:40
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