Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 21b5c47e authored by Eric Laurent's avatar Eric Laurent
Browse files

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
parent bce8effe

services/audioflinger/AudioFlinger.cpp

+24 −14
Original line number Diff line number Diff line
@@ -3907,8 +3907,6 @@ bool AudioFlinger::RecordThread::threadLoop() 
            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 @@ bool AudioFlinger::RecordThread::threadLoop() 
                // clear the overflow.

                usleep(kRecordThreadSleepUs);

            }

        } else {

            unlockEffectChains(effectChains);

        }

        // enable changes in effect chain

        unlockEffectChains(effectChains);

        effectChains.clear();

    }
@@ -5581,13 +5579,11 @@ size_t AudioFlinger::EffectModule::removeHandle(const wp<EffectHandle>& handle) 
        }

    }



    // 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 @@ void AudioFlinger::EffectModule::updateState() { 
            mState = IDLE;

        }

        break;

    default: //IDLE , ACTIVE

    default: //IDLE , ACTIVE, DESTROYED

        break;

    }

}
@@ -5646,7 +5642,7 @@ void AudioFlinger::EffectModule::process() 
{

    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 @@ status_t AudioFlinger::EffectModule::start_l() 
    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 @@ status_t AudioFlinger::EffectModule::command(uint32_t cmdCode, 
    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 @@ status_t AudioFlinger::EffectModule::setEnabled(bool enabled) 
        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 @@ bool AudioFlinger::EffectModule::isEnabled() 
    case IDLE:

    case STOPPING:

    case STOPPED:

    case DESTROYED:

    default:

        return false;

    }
@@ -5943,6 +5948,7 @@ bool AudioFlinger::EffectModule::isProcessEnabled() 
        return true;

    case IDLE:

    case STARTING:

    case DESTROYED:

    default:

        return false;

    }
@@ -6544,6 +6550,10 @@ size_t AudioFlinger::EffectChain::removeEffect_l(const sp<EffectModule>& effect) 


    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 {

services/audioflinger/AudioFlinger.h

+3 −1
Original line number Diff line number Diff line
@@ -995,7 +995,8 @@ private: 
            STARTING,

            ACTIVE,

            STOPPING,

            STOPPED

            STOPPED,

            DESTROYED

        };



        int         id() { return mId; }
@@ -1040,6 +1041,7 @@ private: 
        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);