[raudio] Fix 3664: crash in raudio from multithreading issues #3907
Conversation
First it needs to be taken out of the processing chain, then it can be released. The inverse of the initialization.
First it needs to be taken out of the processing chain, then it can be released. The inverse of the initialization.
This change is twofold: * Add locks to UpdateMusicStream/UpdateAudioStream (second one needed separation) * Remove unnecessary hack to restart music - inlining the statements resulted in a no-op Especially the second part made it easier to ensure thread-safety overall
dertseha
changed the title
| @@ -431,8 +431,10 @@ static bool SaveFileText(const char *fileName, char *text); // Save text | |||
| AudioBuffer *LoadAudioBuffer(ma_format format, ma_uint32 channels, ma_uint32 sampleRate, ma_uint32 sizeInFrames, int usage); | |||
| void UnloadAudioBuffer(AudioBuffer *buffer); | |||
|
|
|||
| bool IsAudioBufferPlayingInLockedState(AudioBuffer *buffer); | |||
There was a problem hiding this comment.
Why are those functions required? Could they just be avoided?
There was a problem hiding this comment.
This is what I mean in the description with
This required separation of some of the functions, as they are called from both threads and mutex is not re-entrant.
Calling lock twice within a call-stack will end up in a deadlock.
There was a problem hiding this comment.
Ok, I see, in that case, could those functions be kept internal to raudio module?
There was a problem hiding this comment.
Those functions are "internal" - or what do you mean? Do you mean to make them static ?
|
@dertseha Thanks for the review, just added a comment |
| @@ -431,8 +431,10 @@ static bool SaveFileText(const char *fileName, char *text); // Save text | |||
| AudioBuffer *LoadAudioBuffer(ma_format format, ma_uint32 channels, ma_uint32 sampleRate, ma_uint32 sizeInFrames, int usage); | |||
| void UnloadAudioBuffer(AudioBuffer *buffer); | |||
|
|
|||
| bool IsAudioBufferPlayingInLockedState(AudioBuffer *buffer); | |||
There was a problem hiding this comment.
Ok, I see, in that case, could those functions be kept internal to raudio module?
|
@dertseha Thanks for the fix, I'm reviewing the exposure of the functions. |
This pull request fixes #3664 .
However, it fixes things slightly different than originally thought/intended.
The triggering issue with
UpdateMusicStream()was in as much mitigated by removing the entire hack code, as it was a no-op. See comment for details.Included changes:
UpdateMusicStream()andPlayMusicStream()InLockedStatewas added to indicate this.I could run all the audio examples and play with them. Still, this is only a small test for a change like this. Please review carefully.