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

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

DO NOT MERGE - audioflinger: fix deadlock upon AudioRecord creation error 

AudioFlinger:openRecord() should not hold mClientLock when
releasing the local reference on AudioRecord as the destructor will
also lock mClientLock.
Same fix for AudioFlinger::createTrack().
Also make sure that AudioFlinger::createEffect() holds mClientLock
when clearing local reference to the Client in case of error.

Regression introduced by 021cf963

Bug: 15118096.
Change-Id: Ie961c398c8e0460bca9b95e2ee4ce6859316c275
parent e5cfbd53

services/audioflinger/AudioFlinger.cpp

+18 −5
Original line number Original line Diff line number Diff line
@@ -635,8 +635,12 @@ sp<IAudioTrack> AudioFlinger::createTrack( 
    if (lStatus != NO_ERROR) {

    if (lStatus != NO_ERROR) {

        // remove local strong reference to Client before deleting the Track so that the

        // remove local strong reference to Client before deleting the Track so that the

        // Client destructor is called by the TrackBase destructor with mClientLock held

        // Client destructor is called by the TrackBase destructor with mClientLock held

        // Don't hold mClientLock when releasing the reference on the track as the

        // destructor will acquire it.

        {

            Mutex::Autolock _cl(mClientLock);

            Mutex::Autolock _cl(mClientLock);

            client.clear();

            client.clear();

        }

        track.clear();

        track.clear();

        goto Exit;

        goto Exit;

    }

    }
@@ -1173,7 +1177,7 @@ void AudioFlinger::registerClient(const sp<IAudioFlingerClient>& client) 
    }

    }





    // mClientLock should not be held here because ThreadBase::sendIoConfigEvent() will lock the

    // mClientLock should not be held here because ThreadBase::sendIoConfigEvent() will lock the

    // ThreadBase mutex and teh locknig order is ThreadBase::mLock then AudioFlinger::mClientLock.

    // ThreadBase mutex and the locking order is ThreadBase::mLock then AudioFlinger::mClientLock.

    if (clientAdded) {

    if (clientAdded) {

        // 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
@@ -1419,8 +1423,12 @@ sp<IAudioRecord> AudioFlinger::openRecord( 
    if (lStatus != NO_ERROR) {

    if (lStatus != NO_ERROR) {

        // remove local strong reference to Client before deleting the RecordTrack so that the

        // remove local strong reference to Client before deleting the RecordTrack so that the

        // Client destructor is called by the TrackBase destructor with mClientLock held

        // Client destructor is called by the TrackBase destructor with mClientLock held

        // Don't hold mClientLock when releasing the reference on the track as the

        // destructor will acquire it.

        {

            Mutex::Autolock _cl(mClientLock);

            Mutex::Autolock _cl(mClientLock);

            client.clear();

            client.clear();

        }

        recordTrack.clear();

        recordTrack.clear();

        goto Exit;

        goto Exit;

    }

    }
@@ -2380,6 +2388,11 @@ sp<IEffect> AudioFlinger::createEffect( 
        if (handle != 0 && id != NULL) {

        if (handle != 0 && id != NULL) {

            *id = handle->id();

            *id = handle->id();

        }

        }

        if (handle == 0) {

            // remove local strong reference to Client with mClientLock held

            Mutex::Autolock _cl(mClientLock);

            client.clear();

        }

    }

    }





Exit:

Exit: