AudioFlinger: fix crash when deleting pre process.
If a pre processing effect is detroyed while enabled and capture is active,
there was a possibility that the effect engine is released by the framework
while still processed by the audio HAL.
The fix consists in not releasing the engine in EffectModule::removeHandle()
but just flag the effect as being detroyed to avoid further calls to functions
on the engine effect interface.
The effect interface is then removed from the audio HAL safely in
EffectChain::removeEffect_l() while holding the EffectChain mutex.
Change-Id: I71fab30d9145062af8644f545a1f1d4d3e7e7f02
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index cb1f921..6403061 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -3907,8 +3907,6 @@
for (size_t i = 0; i < effectChains.size(); i ++) {
effectChains[i]->process_l();
}
- // enable changes in effect chain
- unlockEffectChains(effectChains);
buffer.frameCount = mFrameCount;
if (LIKELY(mActiveTrack->getNextBuffer(&buffer) == NO_ERROR)) {
@@ -4008,9 +4006,9 @@
// clear the overflow.
usleep(kRecordThreadSleepUs);
}
- } else {
- unlockEffectChains(effectChains);
}
+ // enable changes in effect chain
+ unlockEffectChains(effectChains);
effectChains.clear();
}
@@ -5581,13 +5579,11 @@
}
}
- // Release effect engine here so that it is done immediately. Otherwise it will be released
- // by the destructor when the last strong reference on the this object is released which can
- // happen after next process is called on this effect.
- if (size == 0 && mEffectInterface != NULL) {
- // release effect engine
- EffectRelease(mEffectInterface);
- mEffectInterface = NULL;
+ // Prevent calls to process() and other functions on effect interface from now on.
+ // The effect engine will be released by the destructor when the last strong reference on
+ // this object is released which can happen after next process is called.
+ if (size == 0) {
+ mState = DESTROYED;
}
return size;
@@ -5637,7 +5633,7 @@
mState = IDLE;
}
break;
- default: //IDLE , ACTIVE
+ default: //IDLE , ACTIVE, DESTROYED
break;
}
}
@@ -5646,7 +5642,7 @@
{
Mutex::Autolock _l(mLock);
- if (mEffectInterface == NULL ||
+ if (mState == DESTROYED || mEffectInterface == NULL ||
mConfig.inputCfg.buffer.raw == NULL ||
mConfig.outputCfg.buffer.raw == NULL) {
return;
@@ -5822,6 +5818,12 @@
return status;
}
+status_t AudioFlinger::EffectModule::stop()
+{
+ Mutex::Autolock _l(mLock);
+ return stop_l();
+}
+
status_t AudioFlinger::EffectModule::stop_l()
{
if (mEffectInterface == NULL) {
@@ -5858,7 +5860,7 @@
Mutex::Autolock _l(mLock);
// LOGV("command(), cmdCode: %d, mEffectInterface: %p", cmdCode, mEffectInterface);
- if (mEffectInterface == NULL) {
+ if (mState == DESTROYED || mEffectInterface == NULL) {
return NO_INIT;
}
status_t status = (*mEffectInterface)->command(mEffectInterface,
@@ -5907,6 +5909,8 @@
case ACTIVE:
mState = STOPPING;
break;
+ case DESTROYED:
+ return NO_ERROR; // simply ignore as we are being destroyed
}
for (size_t i = 1; i < mHandles.size(); i++) {
sp<EffectHandle> h = mHandles[i].promote();
@@ -5928,6 +5932,7 @@
case IDLE:
case STOPPING:
case STOPPED:
+ case DESTROYED:
default:
return false;
}
@@ -5943,6 +5948,7 @@
return true;
case IDLE:
case STARTING:
+ case DESTROYED:
default:
return false;
}
@@ -6544,6 +6550,10 @@
for (i = 0; i < size; i++) {
if (effect == mEffects[i]) {
+ // calling stop here will remove pre-processing effect from the audio HAL.
+ // This is safe as we hold the EffectChain mutex which guarantees that we are not in
+ // the middle of a read from audio HAL
+ mEffects[i]->stop();
if (type == EFFECT_FLAG_TYPE_AUXILIARY) {
delete[] effect->inBuffer();
} else {
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index e2cf946..9817c90 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -995,7 +995,8 @@
STARTING,
ACTIVE,
STOPPING,
- STOPPED
+ STOPPED,
+ DESTROYED
};
int id() { return mId; }
@@ -1040,6 +1041,7 @@
status_t setDevice(uint32_t device);
status_t setVolume(uint32_t *left, uint32_t *right, bool controller);
status_t setMode(uint32_t mode);
+ status_t stop();
status_t dump(int fd, const Vector<String16>& args);
