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 deadlock in voice callbacks #342

Merged
merged 2 commits into from
May 11, 2024
Merged

Conversation

xtsm
Copy link
Contributor

@xtsm xtsm commented Apr 21, 2024

Second attempt. Previous here: #337

Now that we release buffer lock before executing callbacks, some code from another thread can get scheduled between mutex unlock and callback call and observe empty buffer queue right before callback puts a new buffer there. XNA_Song determines the end of the song by simply checking if the queue is empty, which may lead to random playback breaks.

Not sure how to properly check for the song end but the buffer decoding function also zeroes out total samples counter when it finishes a EOS-flagged buffer so checking if that counter is zero should cut it... probably?

xtsm and others added 2 commits April 21, 2024 17:34
Deadlock was being caused by the fact that some private FAudio routines
hold internal mutexes while calling application callbacks. This may result
in a deadlock because app might have a huge global mutex that is locked
both by callbacks and by some code that e.g. submits new buffers to XAudio.

Should fix https://bugs.winehq.org/show_bug.cgi?id=54246
Now that we release buffer lock before executing callbacks, some code
from another thread can get scheduled between mutex unlock and callback
call and observe empty buffer queue right before callback puts a new
buffer there. XNA_Song determines the end of the song by simply checking
if the queue is empty, which may lead to random playback breaks.

Not sure how to properly check for the song end but the buffer decoding
function also zeroes out total samples counter when it finishes a
EOS-flagged buffer so checking if that counter is zero should cut it...
probably?
@xtsm
Copy link
Contributor Author

xtsm commented Apr 21, 2024

@Romans-I-XVI could you please check if the problem you reported earlier (#337 (comment)) persists in this branch?

@Romans-I-XVI
Copy link

@Romans-I-XVI could you please check if the problem you reported earlier (#337 (comment)) persists in this branch?

Just tested it out, this fixes the issue I was experiencing.

@flibitijibibo
Copy link
Member

Sounds good - we're pretty close to the tag date so I'll merge this in right after 24.05 is out!

@xtsm
Copy link
Contributor Author

xtsm commented May 11, 2024

@flibitijibibo no tag yet? i thought FNA 24.05 already got released

@flibitijibibo
Copy link
Member

Fell behind due to SDL_GPU and some other things, will do a full review later today to be extra sure that this is solid.

Copy link
Member

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

We should probably isolate the callbacks in such a way that we won't have to copypaste these unlock/lock pairs so much... but that's for another day.

@flibitijibibo flibitijibibo merged commit f68e150 into FNA-XNA:master May 11, 2024
5 checks passed
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.

3 participants