[Bug 761083] New: Core Audio deadlock

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=761083

            Bug ID: 761083
           Summary: Core Audio deadlock
    Classification: Platform
           Product: GStreamer
           Version: git master
                OS: Mac OS
            Status: NEW
          Severity: normal
          Priority: Normal
         Component: gst-plugins-good
          Assignee: gstreamer-bugs at lists.freedesktop.org
          Reporter: alexander.lenhardt at soundcloud.com
        QA Contact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---

Created attachment 319680
  --> https://bugzilla.gnome.org/attachment.cgi?id=319680&action=edit
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.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.