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

Commit 4f0f17d2 authored by Eric Laurent's avatar Eric Laurent
Browse files

Fix issue 2678048: binder death detection in AudioFlinger is broken. 

There is a bug in the way notification client list is managed when the client binder
interface dies that makes that the dead client is not removed from the list: the week
reference passed by binderDied() cannot be promoted and compared to the strong
references in the list.

The fix consists in creating a new NotificationClient class that implements the
binder DeathRecipient and holds a strong reference to the IAudioFlingerClient interface.
A new instance of this class is created for each cient and a strong reference to this
object is added to the notification client list maintained by AudioFlinger.
When binderDied() is called on this object, it is removed from the list preventing
AudioFlinger to notify this client for further io changes.

Also added code to disable LifeVibes effects when the client that has enabled the
enhancements dies.

Change-Id: Icedc4af171759e9ae9a575d82d44784b4e8267e8
parent 2cd841d4

libs/audioflinger/AudioFlinger.cpp

+62 −30
Original line number Original line Diff line number Diff line
@@ -142,6 +142,7 @@ AudioFlinger::AudioFlinger() 
    }

    }

#ifdef LVMX

#ifdef LVMX

    LifeVibes::init();

    LifeVibes::init();

    mLifeVibesClientPid = -1;

#endif

#endif

}

}
@@ -596,8 +597,10 @@ status_t AudioFlinger::setParameters(int ioHandle, const String8& keyValuePairs) 
    int musicEnabled = -1;

    int musicEnabled = -1;

    if (NO_ERROR == param.get(key, value)) {

    if (NO_ERROR == param.get(key, value)) {

        if (value == LifevibesEnable) {

        if (value == LifevibesEnable) {

            mLifeVibesClientPid = IPCThreadState::self()->getCallingPid();

            musicEnabled = 1;

            musicEnabled = 1;

        } else if (value == LifevibesDisable) {

        } else if (value == LifevibesDisable) {

            mLifeVibesClientPid = -1;

            musicEnabled = 0;

            musicEnabled = 0;

        }

        }

    }

    }
@@ -609,7 +612,7 @@ status_t AudioFlinger::setParameters(int ioHandle, const String8& keyValuePairs) 
        mHardwareStatus = AUDIO_SET_PARAMETER;

        mHardwareStatus = AUDIO_SET_PARAMETER;

        result = mAudioHardware->setParameters(keyValuePairs);

        result = mAudioHardware->setParameters(keyValuePairs);

#ifdef LVMX

#ifdef LVMX

        if ((NO_ERROR == result) && (musicEnabled != -1)) {

        if (musicEnabled != -1) {

            LifeVibes::enableMusic((bool) musicEnabled);

            LifeVibes::enableMusic((bool) musicEnabled);

        }

        }

#endif

#endif
@@ -713,15 +716,19 @@ status_t AudioFlinger::getRenderPosition(uint32_t *halFrames, uint32_t *dspFrame 
void AudioFlinger::registerClient(const sp<IAudioFlingerClient>& client)

void AudioFlinger::registerClient(const sp<IAudioFlingerClient>& client)

{

{





    LOGV("registerClient() %p, tid %d, calling tid %d", client.get(), gettid(), IPCThreadState::self()->getCallingPid());

    Mutex::Autolock _l(mLock);

    Mutex::Autolock _l(mLock);





    int pid = IPCThreadState::self()->getCallingPid();

    if (mNotificationClients.indexOfKey(pid) < 0) {

        sp<NotificationClient> notificationClient = new NotificationClient(this,

                                                                            client,

                                                                            pid);

        LOGV("registerClient() client %p, pid %d", notificationClient.get(), pid);



        mNotificationClients.add(pid, notificationClient);



        sp<IBinder> binder = client->asBinder();

        sp<IBinder> binder = client->asBinder();

    if (mNotificationClients.indexOf(binder) < 0) {

        binder->linkToDeath(notificationClient);

        LOGV("Adding notification client %p", binder.get());

        binder->linkToDeath(this);

        mNotificationClients.add(binder);

    }





        // the config change is always sent from playback or record threads to avoid deadlock

        // the config change is always sent from playback or record threads to avoid deadlock

        // with AudioSystem::gLock

        // with AudioSystem::gLock
@@ -733,31 +740,33 @@ void AudioFlinger::registerClient(const sp<IAudioFlingerClient>& client) 
            mRecordThreads.valueAt(i)->sendConfigEvent(AudioSystem::INPUT_OPENED);

            mRecordThreads.valueAt(i)->sendConfigEvent(AudioSystem::INPUT_OPENED);

        }

        }

    }

    }

}





void AudioFlinger::binderDied(const wp<IBinder>& who) {

void AudioFlinger::removeNotificationClient(pid_t pid)



{

    LOGV("binderDied() %p, tid %d, calling tid %d", who.unsafe_get(), gettid(), IPCThreadState::self()->getCallingPid());

    Mutex::Autolock _l(mLock);

    Mutex::Autolock _l(mLock);





    IBinder *binder = who.unsafe_get();

    int index = mNotificationClients.indexOfKey(pid);



    if (binder != NULL) {

        int index = mNotificationClients.indexOf(binder);

    if (index >= 0) {

    if (index >= 0) {

            LOGV("Removing notification client %p", binder);

        sp <NotificationClient> client = mNotificationClients.valueFor(pid);

            mNotificationClients.removeAt(index);

        LOGV("removeNotificationClient() %p, pid %d", client.get(), pid);

#ifdef LVMX

        if (pid == mLifeVibesClientPid) {

            LOGV("Disabling lifevibes");

            LifeVibes::enableMusic(false);

            mLifeVibesClientPid = -1;

        }

        }

#endif

        mNotificationClients.removeItem(pid);

    }

    }

}

}





// audioConfigChanged_l() must be called with AudioFlinger::mLock held

// audioConfigChanged_l() must be called with AudioFlinger::mLock held

void AudioFlinger::audioConfigChanged_l(int event, int ioHandle, void *param2) {

void AudioFlinger::audioConfigChanged_l(int event, int ioHandle, void *param2)

{

    size_t size = mNotificationClients.size();

    size_t size = mNotificationClients.size();

    for (size_t i = 0; i < size; i++) {

    for (size_t i = 0; i < size; i++) {

        sp<IBinder> binder = mNotificationClients.itemAt(i);

        mNotificationClients.valueAt(i)->client()->ioConfigChanged(event, ioHandle, param2);

        LOGV("audioConfigChanged_l() Notifying change to client %p", binder.get());

        sp<IAudioFlingerClient> client = interface_cast<IAudioFlingerClient> (binder);

        client->ioConfigChanged(event, ioHandle, param2);

    }

    }

}

}
@@ -768,6 +777,7 @@ void AudioFlinger::removeClient_l(pid_t pid) 
    mClients.removeItem(pid);

    mClients.removeItem(pid);

}

}







// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------





AudioFlinger::ThreadBase::ThreadBase(const sp<AudioFlinger>& audioFlinger, int id)

AudioFlinger::ThreadBase::ThreadBase(const sp<AudioFlinger>& audioFlinger, int id)
@@ -3086,6 +3096,28 @@ const sp<MemoryDealer>& AudioFlinger::Client::heap() const 




// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------





AudioFlinger::NotificationClient::NotificationClient(const sp<AudioFlinger>& audioFlinger,

                                                     const sp<IAudioFlingerClient>& client,

                                                     pid_t pid)

    : mAudioFlinger(audioFlinger), mPid(pid), mClient(client)

{

}



AudioFlinger::NotificationClient::~NotificationClient()

{

    mClient.clear();

}



void AudioFlinger::NotificationClient::binderDied(const wp<IBinder>& who)

{

    sp<NotificationClient> keep(this);

    {

        mAudioFlinger->removeNotificationClient(mPid);

    }

}



// ----------------------------------------------------------------------------



AudioFlinger::TrackHandle::TrackHandle(const sp<AudioFlinger::PlaybackThread::Track>& track)

AudioFlinger::TrackHandle::TrackHandle(const sp<AudioFlinger::PlaybackThread::Track>& track)

    : BnAudioTrack(),

    : BnAudioTrack(),

      mTrack(track)

      mTrack(track)

libs/audioflinger/AudioFlinger.h

+27 −5
Original line number Original line Diff line number Diff line
@@ -57,7 +57,7 @@ class AudioResampler; 




static const nsecs_t kStandbyTimeInNsecs = seconds(3);

static const nsecs_t kStandbyTimeInNsecs = seconds(3);





class AudioFlinger : public BnAudioFlinger, public IBinder::DeathRecipient

class AudioFlinger : public BnAudioFlinger

{

{

public:

public:

    static void instantiate();

    static void instantiate();
@@ -139,9 +139,6 @@ public: 




    virtual status_t getRenderPosition(uint32_t *halFrames, uint32_t *dspFrames, int output);

    virtual status_t getRenderPosition(uint32_t *halFrames, uint32_t *dspFrames, int output);





    // IBinder::DeathRecipient

    virtual     void        binderDied(const wp<IBinder>& who);



    enum hardware_call_state {

    enum hardware_call_state {

        AUDIO_HW_IDLE = 0,

        AUDIO_HW_IDLE = 0,

        AUDIO_HW_INIT,

        AUDIO_HW_INIT,
@@ -205,6 +202,27 @@ private: 
        pid_t               mPid;

        pid_t               mPid;

    };

    };





    // --- Notification Client ---

    class NotificationClient : public IBinder::DeathRecipient {

    public:

                            NotificationClient(const sp<AudioFlinger>& audioFlinger,

                                                const sp<IAudioFlingerClient>& client,

                                                pid_t pid);

        virtual             ~NotificationClient();



                sp<IAudioFlingerClient>    client() { return mClient; }



                // IBinder::DeathRecipient

                virtual     void        binderDied(const wp<IBinder>& who);



    private:

                            NotificationClient(const NotificationClient&);

                            NotificationClient& operator = (const NotificationClient&);



        sp<AudioFlinger>        mAudioFlinger;

        pid_t                   mPid;

        sp<IAudioFlingerClient> mClient;

    };





    class TrackHandle;

    class TrackHandle;

    class RecordHandle;

    class RecordHandle;
@@ -685,6 +703,7 @@ private: 








                void        removeClient_l(pid_t pid);

                void        removeClient_l(pid_t pid);

                void        removeNotificationClient(pid_t pid);









    // record thread

    // record thread
@@ -796,8 +815,11 @@ private: 




                DefaultKeyedVector< int, sp<RecordThread> >    mRecordThreads;

                DefaultKeyedVector< int, sp<RecordThread> >    mRecordThreads;





                SortedVector< sp<IBinder> >         mNotificationClients;

                DefaultKeyedVector< pid_t, sp<NotificationClient> >    mNotificationClients;

                int                                 mNextThreadId;

                int                                 mNextThreadId;

#ifdef LVMX

                int mLifeVibesClientPid;

#endif

};

};





// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------