audio HAL: reverse mutex locking order

Do not use main audio HAL mutex but a specific stream mutex when
preventing control thread starvation by playback or capture threads.
This will prevent systematic locking of main HAL mutex in read ot write
and avoid glitches.

Bug: 21880828.
Bug: 18489202.

Change-Id: I3054e0d93b823bd9d2fb84a49e9c5bbbe728262a

1 files changed, 51 insertions, 26 deletions

diff --git a/modules/usbaudio/audio_hal.c b/modules/usbaudio/audio_hal.c
index 872fa93..bbea5f5 100644
--- a/modules/usbaudio/audio_hal.c
+++ b/modules/usbaudio/audio_hal.c
@@ -80,6 +80,7 @@ struct stream_out {
     struct audio_stream_out stream;
 
     pthread_mutex_t lock;               /* see note below on mutex acquisition order */
+    pthread_mutex_t pre_lock;           /* acquire before lock to avoid DOS by playback thread */
     bool standby;
 
     struct audio_device *dev;           /* hardware information - only using this for the lock */
@@ -103,7 +104,8 @@ struct stream_out {
 struct stream_in {
     struct audio_stream_in stream;
 
-    pthread_mutex_t lock; /* see note below on mutex acquisition order */
+    pthread_mutex_t lock;               /* see note below on mutex acquisition order */
+    pthread_mutex_t pre_lock;           /* acquire before lock to avoid DOS by capture thread */
     bool standby;
 
     struct audio_device *dev;           /* hardware information - only using this for the lock */
@@ -126,6 +128,13 @@ struct stream_in {
 };
 
 /*
+ * NOTE: when multiple mutexes have to be acquired, always take the
+ * stream_in or stream_out mutex first, followed by the audio_device mutex.
+ * stream pre_lock is always acquired before stream lock to prevent starvation of control thread by
+ * higher priority playback or capture thread.
+ */
+
+/*
  * Extract the card and device numbers from the supplied key/value pairs.
  *   kvpairs    A null-terminated string containing the key/value pairs or card and device.
  *              i.e. "card=1;device=42"
@@ -203,6 +212,20 @@ static char * device_get_parameters(alsa_device_profile * profile, const char *
     return result_str;
 }
 
+void lock_input_stream(struct stream_in *in)
+{
+    pthread_mutex_lock(&in->pre_lock);
+    pthread_mutex_lock(&in->lock);
+    pthread_mutex_unlock(&in->pre_lock);
+}
+
+void lock_output_stream(struct stream_out *out)
+{
+    pthread_mutex_lock(&out->pre_lock);
+    pthread_mutex_lock(&out->lock);
+    pthread_mutex_unlock(&out->pre_lock);
+}
+
 /*
  * HAl Functions
  */
@@ -260,16 +283,14 @@ static int out_standby(struct audio_stream *stream)
 {
     struct stream_out *out = (struct stream_out *)stream;
 
-    pthread_mutex_lock(&out->dev->lock);
-    pthread_mutex_lock(&out->lock);
-
+    lock_output_stream(out);
     if (!out->standby) {
+        pthread_mutex_lock(&out->dev->lock);
         proxy_close(&out->proxy);
+        pthread_mutex_unlock(&out->dev->lock);
         out->standby = true;
     }
-
     pthread_mutex_unlock(&out->lock);
-    pthread_mutex_unlock(&out->dev->lock);
 
     return 0;
 }
@@ -295,9 +316,9 @@ static int out_set_parameters(struct audio_stream *stream, const char *kvpairs)
         return ret_value;
     }
 
+    lock_output_stream(out);
     /* Lock the device because that is where the profile lives */
     pthread_mutex_lock(&out->dev->lock);
-    pthread_mutex_lock(&out->lock);
 
     if (!profile_is_cached_for(out->profile, card, device)) {
         /* cannot read pcm device info if playback is active */
@@ -316,8 +337,8 @@ static int out_set_parameters(struct audio_stream *stream, const char *kvpairs)
         }
     }
 
-    pthread_mutex_unlock(&out->lock);
     pthread_mutex_unlock(&out->dev->lock);
+    pthread_mutex_unlock(&out->lock);
 
     return ret_value;
 }
@@ -325,8 +346,8 @@ static int out_set_parameters(struct audio_stream *stream, const char *kvpairs)
 static char * out_get_parameters(const struct audio_stream *stream, const char *keys)
 {
     struct stream_out *out = (struct stream_out *)stream;
+    lock_output_stream(out);
     pthread_mutex_lock(&out->dev->lock);
-    pthread_mutex_lock(&out->lock);
 
     char * params_str =  device_get_parameters(out->profile, keys);
 
@@ -360,17 +381,16 @@ static ssize_t out_write(struct audio_stream_out *stream, const void* buffer, si
     int ret;
     struct stream_out *out = (struct stream_out *)stream;
 
-    pthread_mutex_lock(&out->dev->lock);
-    pthread_mutex_lock(&out->lock);
+    lock_output_stream(out);
     if (out->standby) {
+        pthread_mutex_lock(&out->dev->lock);
         ret = start_output_stream(out);
+        pthread_mutex_unlock(&out->dev->lock);
         if (ret != 0) {
-            pthread_mutex_unlock(&out->dev->lock);
             goto err;
         }
         out->standby = false;
     }
-    pthread_mutex_unlock(&out->dev->lock);
 
     alsa_device_proxy* proxy = &out->proxy;
     const void * write_buff = buffer;
@@ -480,6 +500,9 @@ static int adev_open_output_stream(struct audio_hw_device *dev,
     out->stream.get_presentation_position = out_get_presentation_position;
     out->stream.get_next_write_timestamp = out_get_next_write_timestamp;
 
+    pthread_mutex_init(&out->lock, (const pthread_mutexattr_t *) NULL);
+    pthread_mutex_init(&out->pre_lock, (const pthread_mutexattr_t *) NULL);
+
     out->dev = adev;
     pthread_mutex_lock(&adev->lock);
     out->profile = &adev->out_profile;
@@ -639,16 +662,15 @@ static int in_standby(struct audio_stream *stream)
 {
     struct stream_in *in = (struct stream_in *)stream;
 
-    pthread_mutex_lock(&in->dev->lock);
-    pthread_mutex_lock(&in->lock);
-
+    lock_input_stream(in);
     if (!in->standby) {
+        pthread_mutex_lock(&in->dev->lock);
         proxy_close(&in->proxy);
+        pthread_mutex_unlock(&in->dev->lock);
         in->standby = true;
     }
 
     pthread_mutex_unlock(&in->lock);
-    pthread_mutex_unlock(&in->dev->lock);
 
     return 0;
 }
@@ -676,8 +698,8 @@ static int in_set_parameters(struct audio_stream *stream, const char *kvpairs)
         return ret_value;
     }
 
+    lock_input_stream(in);
     pthread_mutex_lock(&in->dev->lock);
-    pthread_mutex_lock(&in->lock);
 
     if (card >= 0 && device >= 0 && !profile_is_cached_for(in->profile, card, device)) {
         /* cannot read pcm device info if playback is active */
@@ -696,8 +718,8 @@ static int in_set_parameters(struct audio_stream *stream, const char *kvpairs)
         }
     }
 
-    pthread_mutex_unlock(&in->lock);
     pthread_mutex_unlock(&in->dev->lock);
+    pthread_mutex_unlock(&in->lock);
 
     return ret_value;
 }
@@ -706,13 +728,13 @@ static char * in_get_parameters(const struct audio_stream *stream, const char *k
 {
     struct stream_in *in = (struct stream_in *)stream;
 
+    lock_input_stream(in);
     pthread_mutex_lock(&in->dev->lock);
-    pthread_mutex_lock(&in->lock);
 
     char * params_str =  device_get_parameters(in->profile, keys);
 
-    pthread_mutex_unlock(&in->lock);
     pthread_mutex_unlock(&in->dev->lock);
+    pthread_mutex_unlock(&in->lock);
 
     return params_str;
 }
@@ -750,16 +772,16 @@ static ssize_t in_read(struct audio_stream_in *stream, void* buffer, size_t byte
 
     struct stream_in * in = (struct stream_in *)stream;
 
-    pthread_mutex_lock(&in->dev->lock);
-    pthread_mutex_lock(&in->lock);
+    lock_input_stream(in);
     if (in->standby) {
-        if (start_input_stream(in) != 0) {
-            pthread_mutex_unlock(&in->dev->lock);
+        pthread_mutex_lock(&in->dev->lock);
+        ret = start_input_stream(in);
+        pthread_mutex_unlock(&in->dev->lock);
+        if (ret != 0) {
             goto err;
         }
         in->standby = false;
     }
-    pthread_mutex_unlock(&in->dev->lock);
 
     alsa_device_profile * profile = in->profile;
 
@@ -858,6 +880,9 @@ static int adev_open_input_stream(struct audio_hw_device *dev,
     in->stream.read = in_read;
     in->stream.get_input_frames_lost = in_get_input_frames_lost;
 
+    pthread_mutex_init(&in->lock, (const pthread_mutexattr_t *) NULL);
+    pthread_mutex_init(&in->pre_lock, (const pthread_mutexattr_t *) NULL);
+
     in->dev = (struct audio_device *)dev;
     pthread_mutex_lock(&in->dev->lock);