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.
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 @@
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 @@
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 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 @@
}
}
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 @@
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 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 @@
}
}
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 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 @@
}
}
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 @@
}
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 @@
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 @@
int mFormat;
uint32_t mFrameSize;
Condition mParamCond;
- String8 mNewParameters;
+ Vector<String8> mNewParameters;
status_t mParamStatus;
Vector<ConfigEvent *> mConfigEvents;
bool mStandby;