After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 761083 - Core Audio deadlock
Core Audio deadlock
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Mac OS
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-01-25 13:17 UTC by Alexander Lenhardt
Modified: 2018-11-03 15:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix deadlock in core audio sink (4.01 KB, patch)
2016-01-25 13:17 UTC, Alexander Lenhardt
reviewed Details | Review

Description Alexander Lenhardt 2016-01-25 13:17:05 UTC
Created attachment 319680 [details] [review]
Fix deadlock in core audio sink

Basically any call to AudioUnitSetProperty/AudioUnitGetProperty from within the audio thread can and will cause a deadlock at some point.
If you need to do that, then it should be used through the AUGraph wrapper which adds thread safety.
When the audiounit render callback is executing, it enters through HALB_IOThread::Entry. Looking at the disassembly shows that this function locks the HALB_Mutex. 
Now, concurrently 1 or more other threads can request to close the graph while it is running the render callback. 
Closing the graph calls into AUMethodStop which doesnt have a symbolized disassembly, but I found the code in a java binding which shows that it calls AUI_LOCK which is exactly #define AUI_LOCK CAMutex::Locker auLock(AUI->GetMutex()); 
After that, the HALB module tries to lock the HALB_mutex which has been locked before by our render callback.

Audio thread:  ---> HALB_IOThread::Entry ---> lock HALB mutex ---------> do some processing ----> lock AUI mutex [DEADLOCK]
Main thread:   ------------------> stop requested ----> AUMethodStop ---> lock AUI mutex -----> StopIOProc ---> lock on HALB mutex [DEADLOCK]
------------------------------------------------ time -------------------------------------------------------------------------------------->

E.g. a stop() call triggers a render_notify which will try to remove the callback and in turn calls into AudioUnitSetProperty. 
If currently a render callback was executing, then this will cause a deadlock in the way i tried to sketch above with my 1337 ascii drawing skills.

I worked around that by adding a mutex to synchronize render callbacks and calls to stop() from the main thread. So far it works but its not the ultimate solution. 
IMO, it should be avoided to call into AudioUnitSetProperty from render callbacks and any callbacks that are executed in the audio thread (that includes render_notify afaik). This is pretty much inline with what is already mentioned in the comments of the respective code parts in gst_core_audio_io_proc_stop() and gst_core_audio_render_notify() in gstosxcoreaudiocommon.c.
I am currently unsure whats the best place to detach the render_notify and io callbacks, but certainly it must be in a function thats called from the main thread.

For reference, the JUCE and WebRTC project had similar issues with the same root cause, i.e. calling AUGet/AUSetProperty from within a rendering callback.
http://www.juce.com/forum/topic/coreaudio-possible-deadlock-when-moving-mavericks
https://bugs.chromium.org/p/webrtc/issues/detail?id=4128

Probably the best solution would be to port it to the AUGraph API which adds thread safety. But the provided patch simply fixes the posiible deadlocks that can occur with the current implementation.
Comment 1 Sebastian Dröge (slomo) 2016-02-16 11:44:54 UTC
Review of attachment 319680 [details] [review]:

Isn't also some change needed in gstosxaudiosink.c here?

::: sys/osxaudio/gstosxcoreaudiocommon.c
@@ +39,3 @@
 
+  if (!G_TRYLOCK (au_render_cb)) {
+    g_thread_yield ();

Why? Will this be called again later so we can remove the callback? Why can't we just block on the mutex?
Comment 2 GStreamer system administrator 2018-11-03 15:07:23 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/253.