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

Commit aaa44478 authored by Eric Laurent's avatar Eric Laurent
Browse files

audioflinger: fix pre processing effect leak

When a capture thread was closed, the effects attached to this thread
were left dangling and the associated effect chain destroyed.
When their last client was disconnected, the effects were not released
properly from the effect library because the destruction process could
not be completed without the effect being attached to a thread.

A similar problem prevented a RecordTrack to be properly released if
its client was destroyed after the capture thread.

The fix consists in allowing the effect or record track to be properly
released even if its parent thread cannot be promoted.

Also save any effect chain still present on a closed capture thread
in case a new client wants to reuse the effects on the same session later.

Bug: 17110064.
Change-Id: I5cd644daa357afd1f3548f9bcb28e6152d95fdb8
parent f0b31e63
Loading
Loading
Loading
Loading
+68 −1
Original line number Original line Diff line number Diff line
@@ -418,6 +418,13 @@ status_t AudioFlinger::dump(int fd, const Vector<String16>& args)
            mRecordThreads.valueAt(i)->dump(fd, args);
            mRecordThreads.valueAt(i)->dump(fd, args);
        }
        }


        // dump orphan effect chains
        if (mOrphanEffectChains.size() != 0) {
            write(fd, "  Orphan Effect Chains\n", strlen("  Orphan Effect Chains\n"));
            for (size_t i = 0; i < mOrphanEffectChains.size(); i++) {
                mOrphanEffectChains.valueAt(i)->dump(fd, args);
            }
        }
        // dump all hardware devs
        // dump all hardware devs
        for (size_t i = 0; i < mAudioHwDevs.size(); i++) {
        for (size_t i = 0; i < mAudioHwDevs.size(); i++) {
            audio_hw_device_t *dev = mAudioHwDevs.valueAt(i)->hwDevice();
            audio_hw_device_t *dev = mAudioHwDevs.valueAt(i)->hwDevice();
@@ -1416,7 +1423,7 @@ sp<IAudioRecord> AudioFlinger::openRecord(
                *sessionId = lSessionId;
                *sessionId = lSessionId;
            }
            }
        }
        }
        ALOGV("openRecord() lSessionId: %d", lSessionId);
        ALOGV("openRecord() lSessionId: %d input %d", lSessionId, input);


        // TODO: the uid should be passed in as a parameter to openRecord
        // TODO: the uid should be passed in as a parameter to openRecord
        recordTrack = thread->createRecordTrack_l(client, sampleRate, format, channelMask,
        recordTrack = thread->createRecordTrack_l(client, sampleRate, format, channelMask,
@@ -2022,6 +2029,16 @@ status_t AudioFlinger::closeInput_nonvirtual(audio_io_handle_t input)
        }
        }


        ALOGV("closeInput() %d", input);
        ALOGV("closeInput() %d", input);
        {
            // If we still have effect chains, it means that a client still holds a handle
            // on at least one effect. We must keep the chain alive in case a new record
            // thread is opened for a new capture on the same session
            Mutex::Autolock _sl(thread->mLock);
            Vector< sp<EffectChain> > effectChains = thread->getEffectChains_l();
            for (size_t i = 0; i < effectChains.size(); i++) {
                putOrphanEffectChain_l(effectChains[i]);
            }
        }
        audioConfigChanged(AudioSystem::INPUT_CLOSED, input, NULL);
        audioConfigChanged(AudioSystem::INPUT_CLOSED, input, NULL);
        mRecordThreads.removeItem(input);
        mRecordThreads.removeItem(input);
    }
    }
@@ -2451,6 +2468,13 @@ sp<IEffect> AudioFlinger::createEffect(
                lStatus = BAD_VALUE;
                lStatus = BAD_VALUE;
                goto Exit;
                goto Exit;
            }
            }
        } else {
            // Check if one effect chain was awaiting for an effect to be created on this
            // session and used it instead of creating a new one.
            sp<EffectChain> chain = getOrphanEffectChain_l((audio_session_t)sessionId);
            if (chain != 0) {
                thread->addEffectChain_l(chain);
            }
        }
        }


        sp<Client> client = registerPid(pid);
        sp<Client> client = registerPid(pid);
@@ -2623,6 +2647,49 @@ void AudioFlinger::onNonOffloadableGlobalEffectEnable()


}
}


status_t AudioFlinger::putOrphanEffectChain_l(const sp<AudioFlinger::EffectChain>& chain)
{
    audio_session_t session = (audio_session_t)chain->sessionId();
    ssize_t index = mOrphanEffectChains.indexOfKey(session);
    ALOGV("putOrphanEffectChain_l session %d index %d", session, index);
    if (index >= 0) {
        ALOGW("putOrphanEffectChain_l chain for session %d already present", session);
        return ALREADY_EXISTS;
    }
    mOrphanEffectChains.add(session, chain);
    return NO_ERROR;
}

sp<AudioFlinger::EffectChain> AudioFlinger::getOrphanEffectChain_l(audio_session_t session)
{
    sp<EffectChain> chain;
    ssize_t index = mOrphanEffectChains.indexOfKey(session);
    ALOGV("getOrphanEffectChain_l session %d index %d", session, index);
    if (index >= 0) {
        chain = mOrphanEffectChains.valueAt(index);
        mOrphanEffectChains.removeItemsAt(index);
    }
    return chain;
}

bool AudioFlinger::updateOrphanEffectChains(const sp<AudioFlinger::EffectModule>& effect)
{
    Mutex::Autolock _l(mLock);
    audio_session_t session = (audio_session_t)effect->sessionId();
    ssize_t index = mOrphanEffectChains.indexOfKey(session);
    ALOGV("updateOrphanEffectChains session %d index %d", session, index);
    if (index >= 0) {
        sp<EffectChain> chain = mOrphanEffectChains.valueAt(index);
        if (chain->removeEffect_l(effect) == 0) {
            ALOGV("updateOrphanEffectChains removing effect chain at index %d", index);
            mOrphanEffectChains.removeItemsAt(index);
        }
        return true;
    }
    return false;
}


struct Entry {
struct Entry {
#define MAX_NAME 32     // %Y%m%d%H%M%S_%d.wav
#define MAX_NAME 32     // %Y%m%d%H%M%S_%d.wav
    char mName[MAX_NAME];
    char mName[MAX_NAME];
+20 −0
Original line number Original line Diff line number Diff line
@@ -569,6 +569,23 @@ private:
                bool isNonOffloadableGlobalEffectEnabled_l();
                bool isNonOffloadableGlobalEffectEnabled_l();
                void onNonOffloadableGlobalEffectEnable();
                void onNonOffloadableGlobalEffectEnable();


                // Store an effect chain to mOrphanEffectChains keyed vector.
                // Called when a thread exits and effects are still attached to it.
                // If effects are later created on the same session, they will reuse the same
                // effect chain and same instances in the effect library.
                // return ALREADY_EXISTS if a chain with the same session already exists in
                // mOrphanEffectChains. Note that this should never happen as there is only one
                // chain for a given session and it is attached to only one thread at a time.
                status_t        putOrphanEffectChain_l(const sp<EffectChain>& chain);
                // Get an effect chain for the specified session in mOrphanEffectChains and remove
                // it if found. Returns 0 if not found (this is the most common case).
                sp<EffectChain> getOrphanEffectChain_l(audio_session_t session);
                // Called when the last effect handle on an effect instance is removed. If this
                // effect belongs to an effect chain in mOrphanEffectChains, the chain is updated
                // and removed from mOrphanEffectChains if it does not contain any effect.
                // Return true if the effect was found in mOrphanEffectChains, false otherwise.
                bool            updateOrphanEffectChains(const sp<EffectModule>& effect);

    class AudioHwDevice {
    class AudioHwDevice {
    public:
    public:
        enum Flags {
        enum Flags {
@@ -713,6 +730,9 @@ private:
                Vector < sp<SyncEvent> > mPendingSyncEvents; // sync events awaiting for a session
                Vector < sp<SyncEvent> > mPendingSyncEvents; // sync events awaiting for a session
                                                             // to be created
                                                             // to be created


                // Effect chains without a valid thread
                DefaultKeyedVector< audio_session_t , sp<EffectChain> > mOrphanEffectChains;

private:
private:
    sp<Client>  registerPid(pid_t pid);    // always returns non-0
    sp<Client>  registerPid(pid_t pid);    // always returns non-0


+24 −4
Original line number Original line Diff line number Diff line
@@ -68,7 +68,8 @@ AudioFlinger::EffectModule::EffectModule(ThreadBase *thread,
      mStatus(NO_INIT), mState(IDLE),
      mStatus(NO_INIT), mState(IDLE),
      // mMaxDisableWaitCnt is set by configure() and not used before then
      // mMaxDisableWaitCnt is set by configure() and not used before then
      // mDisableWaitCnt is set by process() and updateState() and not used before then
      // mDisableWaitCnt is set by process() and updateState() and not used before then
      mSuspended(false)
      mSuspended(false),
      mAudioFlinger(thread->mAudioFlinger)
{
{
    ALOGV("Constructor %p", this);
    ALOGV("Constructor %p", this);
    int lStatus;
    int lStatus;
@@ -197,9 +198,19 @@ size_t AudioFlinger::EffectModule::disconnect(EffectHandle *handle, bool unpinIf
    // destructor before we exit
    // destructor before we exit
    sp<EffectModule> keep(this);
    sp<EffectModule> keep(this);
    {
    {
        if (removeHandle(handle) == 0) {
            if (!isPinned() || unpinIfLast) {
                sp<ThreadBase> thread = mThread.promote();
                sp<ThreadBase> thread = mThread.promote();
                if (thread != 0) {
                if (thread != 0) {
            thread->disconnectEffect(keep, handle, unpinIfLast);
                    Mutex::Autolock _l(thread->mLock);
                    thread->removeEffect_l(this);
                }
                sp<AudioFlinger> af = mAudioFlinger.promote();
                if (af != 0) {
                    af->updateOrphanEffectChains(this);
                }
                AudioSystem::unregisterEffect(mId);
            }
        }
        }
    }
    }
    return mHandles.size();
    return mHandles.size();
@@ -1911,4 +1922,13 @@ bool AudioFlinger::EffectChain::isNonOffloadableEnabled()
    return false;
    return false;
}
}


void AudioFlinger::EffectChain::setThread(const sp<ThreadBase>& thread)
{
    Mutex::Autolock _l(mLock);
    mThread = thread;
    for (size_t i = 0; i < mEffects.size(); i++) {
        mEffects[i]->setThread(thread);
    }
}

}; // namespace android
}; // namespace android
+3 −0
Original line number Original line Diff line number Diff line
@@ -153,6 +153,7 @@ mutable Mutex mLock; // mutex for process, commands and handl
    uint32_t mDisableWaitCnt;       // current process() calls count during disable period.
    uint32_t mDisableWaitCnt;       // current process() calls count during disable period.
    bool     mSuspended;            // effect is suspended: temporarily disabled by framework
    bool     mSuspended;            // effect is suspended: temporarily disabled by framework
    bool     mOffloaded;            // effect is currently offloaded to the audio DSP
    bool     mOffloaded;            // effect is currently offloaded to the audio DSP
    wp<AudioFlinger>    mAudioFlinger;
};
};


// The EffectHandle class implements the IEffect interface. It provides resources
// The EffectHandle class implements the IEffect interface. It provides resources
@@ -347,6 +348,8 @@ protected:


    void clearInputBuffer_l(sp<ThreadBase> thread);
    void clearInputBuffer_l(sp<ThreadBase> thread);


    void setThread(const sp<ThreadBase>& thread);

    wp<ThreadBase> mThread;     // parent mixer thread
    wp<ThreadBase> mThread;     // parent mixer thread
    Mutex mLock;                // mutex protecting effect list
    Mutex mLock;                // mutex protecting effect list
    Vector< sp<EffectModule> > mEffects; // list of effect modules
    Vector< sp<EffectModule> > mEffects; // list of effect modules
+3 −17
Original line number Original line Diff line number Diff line
@@ -1147,21 +1147,6 @@ void AudioFlinger::ThreadBase::setMode(audio_mode_t mode)
    }
    }
}
}


void AudioFlinger::ThreadBase::disconnectEffect(const sp<EffectModule>& effect,
                                                    EffectHandle *handle,
                                                    bool unpinIfLast) {

    Mutex::Autolock _l(mLock);
    ALOGV("disconnectEffect() %p effect %p", this, effect.get());
    // delete the effect module if removing last handle on it
    if (effect->removeHandle(handle) == 0) {
        if (!effect->isPinned() || unpinIfLast) {
            removeEffect_l(effect);
            AudioSystem::unregisterEffect(effect->id());
        }
    }
}

void AudioFlinger::ThreadBase::getAudioPortConfig(struct audio_port_config *config)
void AudioFlinger::ThreadBase::getAudioPortConfig(struct audio_port_config *config)
{
{
    config->type = AUDIO_PORT_TYPE_MIX;
    config->type = AUDIO_PORT_TYPE_MIX;
@@ -2278,7 +2263,7 @@ status_t AudioFlinger::PlaybackThread::addEffectChain_l(const sp<EffectChain>& c
            }
            }
        }
        }
    }
    }

    chain->setThread(this);
    chain->setInBuffer(buffer, ownsBuffer);
    chain->setInBuffer(buffer, ownsBuffer);
    chain->setOutBuffer(reinterpret_cast<int16_t*>(mEffectBufferEnabled
    chain->setOutBuffer(reinterpret_cast<int16_t*>(mEffectBufferEnabled
            ? mEffectBuffer : mSinkBuffer));
            ? mEffectBuffer : mSinkBuffer));
@@ -6188,10 +6173,11 @@ status_t AudioFlinger::RecordThread::addEffectChain_l(const sp<EffectChain>& cha
{
{
    // only one chain per input thread
    // only one chain per input thread
    if (mEffectChains.size() != 0) {
    if (mEffectChains.size() != 0) {
        ALOGW("addEffectChain_l() already one chain %p on thread %p", chain.get(), this);
        return INVALID_OPERATION;
        return INVALID_OPERATION;
    }
    }
    ALOGV("addEffectChain_l() %p on thread %p", chain.get(), this);
    ALOGV("addEffectChain_l() %p on thread %p", chain.get(), this);

    chain->setThread(this);
    chain->setInBuffer(NULL);
    chain->setInBuffer(NULL);
    chain->setOutBuffer(NULL);
    chain->setOutBuffer(NULL);


Loading