[vlc-devel] [PATCH 2/2] auhal: fix data race with VolumeSet

[Thomas Guillem]

VolumeSet (called from any threads) was accessing the au_unit variable,
that is written and setup from the aout stream thread.

This also fixes possible crashes when changing the volume after a
failing Start().
---
 modules/audio_output/auhal.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/modules/audio_output/auhal.c b/modules/audio_output/auhal.c
index c1960766241..a0afefd2a5a 100644
--- a/modules/audio_output/auhal.c
+++ b/modules/audio_output/auhal.c
@@ -119,6 +119,13 @@ typedef struct
      * thread (start, stop, close) needs no protection. */
     vlc_mutex_t                 selected_device_lock;
 
+    /* Synchronizes access to au_unit and f_volume. This is needed by the
+     * VolumeSet callback, that can be called from any threads. The au_unit
+     * variable is written in the start and stop callbacks from the aout
+     * thread. Streams callbacks (play, pause, flush...) doesn't need this lock
+     * to access au_unit since they are called from the aout thread. */
+    vlc_mutex_t                 volume_lock;
+
     float                       f_volume;
 
     bool                        b_ignore_streams_changed_callback;
@@ -907,10 +914,16 @@ VolumeSet(audio_output_t * p_aout, float volume)
     aout_sys_t *p_sys = p_aout->sys;
     OSStatus ostatus = 0;
 
-    if (p_sys->b_digital)
-        return VLC_EGENERIC;
+    vlc_mutex_lock(&p_sys->volume_lock);
 
     p_sys->f_volume = volume;
+
+    if (p_sys->au_unit == NULL)
+    {
+        vlc_mutex_unlock(&p_sys->volume_lock);
+        return VLC_EGENERIC;
+    }
+
     aout_VolumeReport(p_aout, volume);
 
     /* Set volume for output unit */
@@ -921,6 +934,8 @@ VolumeSet(audio_output_t * p_aout, float volume)
                                     volume * volume * volume,
                                     0);
 
+    vlc_mutex_unlock(&p_sys->volume_lock);
+
     if (var_InheritBool(p_aout, "volume-save"))
         config_PutInt("auhal-volume", lroundf(volume * AOUT_VOLUME_DEFAULT));
 
@@ -1403,10 +1418,14 @@ Stop(audio_output_t *p_aout)
 
     if (p_sys->au_unit)
     {
+        vlc_mutex_lock(&p_sys->volume_lock);
+
         AudioOutputUnitStop(p_sys->au_unit);
         au_Uninitialize(p_aout, p_sys->au_unit);
         AudioComponentInstanceDispose(p_sys->au_unit);
         p_sys->au_unit = NULL;
+
+        vlc_mutex_unlock(&p_sys->volume_lock);
     }
     else
     {
@@ -1602,12 +1621,18 @@ Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
     }
     else
     {
+        vlc_mutex_lock(&p_sys->volume_lock);
+
         if (StartAnalog(p_aout, fmt) == VLC_SUCCESS)
         {
+            vlc_mutex_unlock(&p_sys->volume_lock);
+
             msg_Dbg(p_aout, "analog output successfully opened");
             fmt->channel_type = AUDIO_CHANNEL_TYPE_BITMAP;
             return VLC_SUCCESS;
         }
+
+        vlc_mutex_unlock(&p_sys->volume_lock);
     }
 
     msg_Err(p_aout, "opening auhal output failed");
@@ -1687,6 +1712,7 @@ static int Open(vlc_object_t *obj)
 
     vlc_mutex_init(&p_sys->device_list_lock);
     vlc_mutex_init(&p_sys->selected_device_lock);
+    vlc_mutex_init(&p_sys->volume_lock);
     p_sys->b_digital = false;
     p_sys->au_unit = NULL;
     p_sys->b_ignore_streams_changed_callback = false;
-- 
2.30.0