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

Commit 3464c015 authored by Eric Laurent's avatar Eric Laurent
Browse files

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.
parent 119ed307

libs/audioflinger/AudioFlinger.cpp

+45 −23
Original line number Original line Diff line number Diff line
@@ -710,12 +710,14 @@ void AudioFlinger::removeClient(pid_t pid) 
AudioFlinger::ThreadBase::ThreadBase(const sp<AudioFlinger>& audioFlinger)

AudioFlinger::ThreadBase::ThreadBase(const sp<AudioFlinger>& audioFlinger)

    :   Thread(false),

    :   Thread(false),

        mAudioFlinger(audioFlinger), mSampleRate(0), mFrameCount(0), mChannelCount(0),

        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()

AudioFlinger::ThreadBase::~ThreadBase()

{

{

    mParamCond.broadcast();

    mNewParameters.clear();

}

}





void AudioFlinger::ThreadBase::exit()

void AudioFlinger::ThreadBase::exit()
@@ -755,20 +757,28 @@ size_t AudioFlinger::ThreadBase::frameCount() const 




status_t AudioFlinger::ThreadBase::setParameters(const String8& keyValuePairs)

status_t AudioFlinger::ThreadBase::setParameters(const String8& keyValuePairs)

{

{

    status_t result;

    status_t status;





    LOGV("ThreadBase::setParameters() %s", keyValuePairs.string());

    Mutex::Autolock _l(mLock);

    Mutex::Autolock _l(mLock);

    mNewParameters = keyValuePairs;





    mNewParameters.add(keyValuePairs);

    mWaitWorkCV.signal();

    mWaitWorkCV.signal();

    mParamCond.wait(mLock);

    mParamCond.wait(mLock);



    status = mParamStatus;

    return mParamStatus;

    mWaitWorkCV.signal();

    return status;

}

}





void AudioFlinger::ThreadBase::sendConfigEvent(int event, int param)

void AudioFlinger::ThreadBase::sendConfigEvent(int event, int param)

{

{

    Mutex::Autolock _l(mLock);

    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 *configEvent = new ConfigEvent();

    configEvent->mEvent = event;

    configEvent->mEvent = event;

    configEvent->mParam = param;

    configEvent->mParam = param;
@@ -1454,10 +1464,14 @@ bool AudioFlinger::MixerThread::checkForNewParameters_l() 
{

{

    bool reconfig = false;

    bool reconfig = false;





    if (mNewParameters != "") {

    while (!mNewParameters.isEmpty()) {

        status_t status = NO_ERROR;

        status_t status = NO_ERROR;

        AudioParameter param = AudioParameter(mNewParameters);

        String8 keyValuePair = mNewParameters[0];

        AudioParameter param = AudioParameter(keyValuePair);

        int value;

        int value;



        mNewParameters.removeAt(0);



        if (param.getInt(String8(AudioParameter::keySamplingRate), value) == NO_ERROR) {

        if (param.getInt(String8(AudioParameter::keySamplingRate), value) == NO_ERROR) {

            reconfig = true;

            reconfig = true;

        }

        }
@@ -1486,12 +1500,12 @@ bool AudioFlinger::MixerThread::checkForNewParameters_l() 
            }

            }

        }

        }

        if (status == NO_ERROR) {

        if (status == NO_ERROR) {

            status = mOutput->setParameters(mNewParameters);

            status = mOutput->setParameters(keyValuePair);

            if (!mStandby && status == INVALID_OPERATION) {

            if (!mStandby && status == INVALID_OPERATION) {

               mOutput->standby();

               mOutput->standby();

               mStandby = true;

               mStandby = true;

               mBytesWritten = 0;

               mBytesWritten = 0;

               status = mOutput->setParameters(mNewParameters);

               status = mOutput->setParameters(keyValuePair);

            }

            }

            if (status == NO_ERROR && reconfig) {

            if (status == NO_ERROR && reconfig) {

                delete mAudioMixer;

                delete mAudioMixer;
@@ -1502,12 +1516,12 @@ bool AudioFlinger::MixerThread::checkForNewParameters_l() 
                    if (name < 0) break;

                    if (name < 0) break;

                    mTracks[i]->mName = name;

                    mTracks[i]->mName = name;

                }

                }

                sendConfigEvent(AudioSystem::OUTPUT_CONFIG_CHANGED);

                sendConfigEvent_l(AudioSystem::OUTPUT_CONFIG_CHANGED);

            }

            }

        }

        }

        mParamStatus = status;

        mParamStatus = status;

        mNewParameters = "";

        mParamCond.signal();

        mParamCond.signal();

        mWaitWorkCV.wait(mLock);

    }

    }

    return reconfig;

    return reconfig;

}

}
@@ -1759,10 +1773,14 @@ bool AudioFlinger::DirectOutputThread::checkForNewParameters_l() 
{

{

    bool reconfig = false;

    bool reconfig = false;





    if (mNewParameters != "") {

    while (!mNewParameters.isEmpty()) {

        status_t status = NO_ERROR;

        status_t status = NO_ERROR;

        AudioParameter param = AudioParameter(mNewParameters);

        String8 keyValuePair = mNewParameters[0];

        AudioParameter param = AudioParameter(keyValuePair);

        int value;

        int value;



        mNewParameters.removeAt(0);



        if (param.getInt(String8(AudioParameter::keyFrameCount), value) == NO_ERROR) {

        if (param.getInt(String8(AudioParameter::keyFrameCount), value) == NO_ERROR) {

            // do not accept frame count changes if tracks are open as the track buffer

            // 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

            // 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) {

        if (status == NO_ERROR) {

            status = mOutput->setParameters(mNewParameters);

            status = mOutput->setParameters(keyValuePair);

            if (!mStandby && status == INVALID_OPERATION) {

            if (!mStandby && status == INVALID_OPERATION) {

               mOutput->standby();

               mOutput->standby();

               mStandby = true;

               mStandby = true;

               mBytesWritten = 0;

               mBytesWritten = 0;

               status = mOutput->setParameters(mNewParameters);

               status = mOutput->setParameters(keyValuePair);

            }

            }

            if (status == NO_ERROR && reconfig) {

            if (status == NO_ERROR && reconfig) {

                readOutputParameters();

                readOutputParameters();

                sendConfigEvent(AudioSystem::OUTPUT_CONFIG_CHANGED);

                sendConfigEvent_l(AudioSystem::OUTPUT_CONFIG_CHANGED);

            }

            }

        }

        }

        mParamStatus = status;

        mParamStatus = status;

        mNewParameters = "";

        mParamCond.signal();

        mParamCond.signal();

        mWaitWorkCV.wait(mLock);

    }

    }

    return reconfig;

    return reconfig;

}

}
@@ -3100,13 +3118,17 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l() 
{

{

    bool reconfig = false;

    bool reconfig = false;





    if (mNewParameters != "") {

    while (!mNewParameters.isEmpty()) {

        status_t status = NO_ERROR;

        status_t status = NO_ERROR;

        AudioParameter param = AudioParameter(mNewParameters);

        String8 keyValuePair = mNewParameters[0];

        AudioParameter param = AudioParameter(keyValuePair);

        int value;

        int value;

        int reqFormat = mFormat;

        int reqFormat = mFormat;

        int reqSamplingRate = mReqSampleRate;

        int reqSamplingRate = mReqSampleRate;

        int reqChannelCount = mReqChannelCount;

        int reqChannelCount = mReqChannelCount;



        mNewParameters.removeAt(0);



        if (param.getInt(String8(AudioParameter::keySamplingRate), value) == NO_ERROR) {

        if (param.getInt(String8(AudioParameter::keySamplingRate), value) == NO_ERROR) {

            reqSamplingRate = value;

            reqSamplingRate = value;

            reconfig = true;

            reconfig = true;
@@ -3130,10 +3152,10 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l() 
            }

            }

        }

        }

        if (status == NO_ERROR) {

        if (status == NO_ERROR) {

            status = mInput->setParameters(mNewParameters);

            status = mInput->setParameters(keyValuePair);

            if (status == INVALID_OPERATION) {

            if (status == INVALID_OPERATION) {

               mInput->standby();

               mInput->standby();

               status = mInput->setParameters(mNewParameters);

               status = mInput->setParameters(keyValuePair);

            }

            }

            if (reconfig) {

            if (reconfig) {

                if (status == BAD_VALUE &&

                if (status == BAD_VALUE &&
@@ -3144,13 +3166,13 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l() 
                }

                }

                if (status == NO_ERROR) {

                if (status == NO_ERROR) {

                    readInputParameters();

                    readInputParameters();

                    sendConfigEvent(AudioSystem::INPUT_CONFIG_CHANGED);

                    sendConfigEvent_l(AudioSystem::INPUT_CONFIG_CHANGED);

                }

                }

            }

            }

        }

        }

        mNewParameters = "";

        mParamStatus = status;

        mParamStatus = status;

        mParamCond.signal();

        mParamCond.signal();

        mWaitWorkCV.wait(mLock);

    }

    }

    return reconfig;

    return reconfig;

}

}

libs/audioflinger/AudioFlinger.h

+2 −1
Original line number Original line Diff line number Diff line
@@ -318,6 +318,7 @@ private: 
        virtual     String8     getParameters(const String8& keys) = 0;

        virtual     String8     getParameters(const String8& keys) = 0;

        virtual     void        audioConfigChanged(int event, int param = 0) = 0;

        virtual     void        audioConfigChanged(int event, int param = 0) = 0;

                    void        sendConfigEvent(int event, int param = 0);

                    void        sendConfigEvent(int event, int param = 0);

                    void        sendConfigEvent_l(int event, int param = 0);

                    void        processConfigEvents();

                    void        processConfigEvents();





        mutable     Mutex                   mLock;

        mutable     Mutex                   mLock;
@@ -341,7 +342,7 @@ private: 
                    int                     mFormat;

                    int                     mFormat;

                    uint32_t                mFrameSize;

                    uint32_t                mFrameSize;

                    Condition               mParamCond;

                    Condition               mParamCond;

                    String8                 mNewParameters;

                    Vector<String8>         mNewParameters;

                    status_t                mParamStatus;

                    status_t                mParamStatus;

                    Vector<ConfigEvent *>   mConfigEvents;

                    Vector<ConfigEvent *>   mConfigEvents;

                    bool                    mStandby;

                    bool                    mStandby;