Fix lockup in audio flinger threadbase setParameters.

The function checkForNewParameters_l() is called with the ThreadBase mutex mLock locked. In the case where the parameter change implies
an audio parameter modification (e.g. sampling rate) the function sendConfigEvent() is called which tries to lock mLock creating a deadlock.

The fix consists in creating a function equivalent to sendConfigEvent() that must be called with mLock locked and does not lock mLock.

Also added the possibility to have more than one set parameter request pending.

2 files changed, 47 insertions, 24 deletions

diff --git a/libs/audioflinger/AudioFlinger.cpp b/libs/audioflinger/AudioFlinger.cpp
index b34f214..3276cdf 100644
--- a/libs/audioflinger/AudioFlinger.cpp
+++ b/libs/audioflinger/AudioFlinger.cpp
@@ -710,12 +710,14 @@ void AudioFlinger::removeClient(pid_t pid)
 AudioFlinger::ThreadBase::ThreadBase(const sp<AudioFlinger>& audioFlinger)
     :   Thread(false),
         mAudioFlinger(audioFlinger), mSampleRate(0), mFrameCount(0), mChannelCount(0),
-        mFormat(0), mFrameSize(1), mNewParameters(String8("")), mStandby(false)
+        mFormat(0), mFrameSize(1), mStandby(false)
 {
 }
 
 AudioFlinger::ThreadBase::~ThreadBase()
 {
+    mParamCond.broadcast();
+    mNewParameters.clear();
 }
 
 void AudioFlinger::ThreadBase::exit()
@@ -755,20 +757,28 @@ size_t AudioFlinger::ThreadBase::frameCount() const
 
 status_t AudioFlinger::ThreadBase::setParameters(const String8& keyValuePairs)
 {
-    status_t result;
+    status_t status;
 
+    LOGV("ThreadBase::setParameters() %s", keyValuePairs.string());
     Mutex::Autolock _l(mLock);
-    mNewParameters = keyValuePairs;
 
+    mNewParameters.add(keyValuePairs);
     mWaitWorkCV.signal();
     mParamCond.wait(mLock);
-
-    return mParamStatus;
+    status = mParamStatus;
+    mWaitWorkCV.signal();
+    return status;
 }
 
 void AudioFlinger::ThreadBase::sendConfigEvent(int event, int param)
 {
     Mutex::Autolock _l(mLock);
+    sendConfigEvent_l(event, param);
+}
+
+// sendConfigEvent_l() must be called with ThreadBase::mLock held
+void AudioFlinger::ThreadBase::sendConfigEvent_l(int event, int param)
+{
     ConfigEvent *configEvent = new ConfigEvent();
     configEvent->mEvent = event;
     configEvent->mParam = param;
@@ -1454,10 +1464,14 @@ bool AudioFlinger::MixerThread::checkForNewParameters_l()
 {
     bool reconfig = false;
 
-    if (mNewParameters != "") {
+    while (!mNewParameters.isEmpty()) {
         status_t status = NO_ERROR;
-        AudioParameter param = AudioParameter(mNewParameters);
+        String8 keyValuePair = mNewParameters[0];
+        AudioParameter param = AudioParameter(keyValuePair);
         int value;
+
+        mNewParameters.removeAt(0);
+
         if (param.getInt(String8(AudioParameter::keySamplingRate), value) == NO_ERROR) {
             reconfig = true;
         }
@@ -1486,12 +1500,12 @@ bool AudioFlinger::MixerThread::checkForNewParameters_l()
             }
         }
         if (status == NO_ERROR) {
-            status = mOutput->setParameters(mNewParameters);
+            status = mOutput->setParameters(keyValuePair);
             if (!mStandby && status == INVALID_OPERATION) {
                mOutput->standby();
                mStandby = true;
                mBytesWritten = 0;
-               status = mOutput->setParameters(mNewParameters);
+               status = mOutput->setParameters(keyValuePair);
             }
             if (status == NO_ERROR && reconfig) {
                 delete mAudioMixer;
@@ -1502,12 +1516,12 @@ bool AudioFlinger::MixerThread::checkForNewParameters_l()
                     if (name < 0) break;
                     mTracks[i]->mName = name;
                 }
-                sendConfigEvent(AudioSystem::OUTPUT_CONFIG_CHANGED);
+                sendConfigEvent_l(AudioSystem::OUTPUT_CONFIG_CHANGED);
             }
         }
         mParamStatus = status;
-        mNewParameters = "";
         mParamCond.signal();
+        mWaitWorkCV.wait(mLock);
     }
     return reconfig;
 }
@@ -1759,10 +1773,14 @@ bool AudioFlinger::DirectOutputThread::checkForNewParameters_l()
 {
     bool reconfig = false;
 
-    if (mNewParameters != "") {
+    while (!mNewParameters.isEmpty()) {
         status_t status = NO_ERROR;
-        AudioParameter param = AudioParameter(mNewParameters);
+        String8 keyValuePair = mNewParameters[0];
+        AudioParameter param = AudioParameter(keyValuePair);
         int value;
+
+        mNewParameters.removeAt(0);
+
         if (param.getInt(String8(AudioParameter::keyFrameCount), value) == NO_ERROR) {
             // do not accept frame count changes if tracks are open as the track buffer
             // size depends on frame count and correct behavior would not be garantied
@@ -1774,21 +1792,21 @@ bool AudioFlinger::DirectOutputThread::checkForNewParameters_l()
             }
         }
         if (status == NO_ERROR) {
-            status = mOutput->setParameters(mNewParameters);
+            status = mOutput->setParameters(keyValuePair);
             if (!mStandby && status == INVALID_OPERATION) {
                mOutput->standby();
                mStandby = true;
                mBytesWritten = 0;
-               status = mOutput->setParameters(mNewParameters);
+               status = mOutput->setParameters(keyValuePair);
             }
             if (status == NO_ERROR && reconfig) {
                 readOutputParameters();
-                sendConfigEvent(AudioSystem::OUTPUT_CONFIG_CHANGED);
+                sendConfigEvent_l(AudioSystem::OUTPUT_CONFIG_CHANGED);
             }
         }
         mParamStatus = status;
-        mNewParameters = "";
         mParamCond.signal();
+        mWaitWorkCV.wait(mLock);
     }
     return reconfig;
 }
@@ -3100,13 +3118,17 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l()
 {
     bool reconfig = false;
 
-    if (mNewParameters != "") {
+    while (!mNewParameters.isEmpty()) {
         status_t status = NO_ERROR;
-        AudioParameter param = AudioParameter(mNewParameters);
+        String8 keyValuePair = mNewParameters[0];
+        AudioParameter param = AudioParameter(keyValuePair);
         int value;
         int reqFormat = mFormat;
         int reqSamplingRate = mReqSampleRate;
         int reqChannelCount = mReqChannelCount;
+
+        mNewParameters.removeAt(0);
+
         if (param.getInt(String8(AudioParameter::keySamplingRate), value) == NO_ERROR) {
             reqSamplingRate = value;
             reconfig = true;
@@ -3130,10 +3152,10 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l()
             }
         }
         if (status == NO_ERROR) {
-            status = mInput->setParameters(mNewParameters);
+            status = mInput->setParameters(keyValuePair);
             if (status == INVALID_OPERATION) {
                mInput->standby();
-               status = mInput->setParameters(mNewParameters);
+               status = mInput->setParameters(keyValuePair);
             }
             if (reconfig) {
                 if (status == BAD_VALUE &&
@@ -3144,13 +3166,13 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l()
                 }
                 if (status == NO_ERROR) {
                     readInputParameters();
-                    sendConfigEvent(AudioSystem::INPUT_CONFIG_CHANGED);
+                    sendConfigEvent_l(AudioSystem::INPUT_CONFIG_CHANGED);
                 }
             }
         }
-        mNewParameters = "";
         mParamStatus = status;
         mParamCond.signal();
+        mWaitWorkCV.wait(mLock);
     }
     return reconfig;
 }
diff --git a/libs/audioflinger/AudioFlinger.h b/libs/audioflinger/AudioFlinger.h
index 4a4b823..c9f3448 100644
--- a/libs/audioflinger/AudioFlinger.h
+++ b/libs/audioflinger/AudioFlinger.h
@@ -318,6 +318,7 @@ private:
         virtual     String8     getParameters(const String8& keys) = 0;
         virtual     void        audioConfigChanged(int event, int param = 0) = 0;
                     void        sendConfigEvent(int event, int param = 0);
+                    void        sendConfigEvent_l(int event, int param = 0);
                     void        processConfigEvents();
 
         mutable     Mutex                   mLock;
@@ -341,7 +342,7 @@ private:
                     int                     mFormat;
                     uint32_t                mFrameSize;
                     Condition               mParamCond;
-                    String8                 mNewParameters;
+                    Vector<String8>         mNewParameters;
                     status_t                mParamStatus;
                     Vector<ConfigEvent *>   mConfigEvents;
                     bool                    mStandby;