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

Commit 959e2d06 authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files

audioserver: Use timed lock instead of bouncing for dumps 

The technique of bouncing lock attempts does not work well
in the case when another lock has higher priority and acquires
the lock frequently. Timed lock works better in this case
because the acquiring thread is then put in a queue and gets
the lock as soon as another thread releases it. The wait time
in the worst case (deadlock) is the same.

Bug: 118842894
Test: Run VOICE_COMMUNICATION capture + AEC effect, dump audioflinger
Change-Id: Idc4fc2b6f5faf6988979f9354dd92441af33e600
parent c70268aa

services/audioflinger/AudioFlinger.cpp

+2 −9
Original line number Diff line number Diff line
@@ -466,15 +466,8 @@ void AudioFlinger::dumpPermissionDenial(int fd, const Vector<String16>& args __u 


bool AudioFlinger::dumpTryLock(Mutex& mutex)

{

    bool locked = false;

    for (int i = 0; i < kDumpLockRetries; ++i) {

        if (mutex.tryLock() == NO_ERROR) {

            locked = true;

            break;

        }

        usleep(kDumpLockSleepUs);

    }

    return locked;

    status_t err = mutex.timedLock(kDumpLockTimeoutNs);

    return err == NO_ERROR;

}



status_t AudioFlinger::dump(int fd, const Vector<String16>& args)

services/audioflinger/AudioFlinger.h

+1 −2
Original line number Diff line number Diff line
@@ -431,8 +431,7 @@ private: 
    static uint32_t         mScreenState;



    // Internal dump utilities.

    static const int kDumpLockRetries = 50;

    static const int kDumpLockSleepUs = 20000;

    static const int kDumpLockTimeoutNs = 1 * NANOS_PER_SECOND;

    static bool dumpTryLock(Mutex& mutex);

    void dumpPermissionDenial(int fd, const Vector<String16>& args);

    void dumpClients(int fd, const Vector<String16>& args);

services/audiopolicy/service/AudioPolicyService.cpp

+8 −15
Original line number Diff line number Diff line
@@ -22,8 +22,9 @@ 
#define __STDINT_LIMITS

#define __STDC_LIMIT_MACROS

#include <stdint.h>



#include <sys/time.h>



#include <audio_utils/clock.h>

#include <binder/IServiceManager.h>

#include <utils/Log.h>

#include <cutils/properties.h>
@@ -48,8 +49,7 @@ namespace android { 
static const char kDeadlockedString[] = "AudioPolicyService may be deadlocked\n";

static const char kCmdDeadlockedString[] = "AudioPolicyService command thread may be deadlocked\n";



static const int kDumpLockRetries = 50;

static const int kDumpLockSleepUs = 20000;

static const int kDumpLockTimeoutNs = 1 * NANOS_PER_SECOND;



static const nsecs_t kAudioCommandTimeoutNs = seconds(3); // 3 seconds
@@ -376,17 +376,10 @@ void AudioPolicyService::binderDied(const wp<IBinder>& who) { 
            IPCThreadState::self()->getCallingPid());

}



static bool tryLock(Mutex& mutex)

static bool dumpTryLock(Mutex& mutex)

{

    bool locked = false;

    for (int i = 0; i < kDumpLockRetries; ++i) {

        if (mutex.tryLock() == NO_ERROR) {

            locked = true;

            break;

        }

        usleep(kDumpLockSleepUs);

    }

    return locked;

    status_t err = mutex.timedLock(kDumpLockTimeoutNs);

    return err == NO_ERROR;

}



status_t AudioPolicyService::dumpInternals(int fd)
@@ -627,7 +620,7 @@ status_t AudioPolicyService::dump(int fd, const Vector<String16>& args __unused) 
    if (!dumpAllowed()) {

        dumpPermissionDenial(fd);

    } else {

        bool locked = tryLock(mLock);

        bool locked = dumpTryLock(mLock);

        if (!locked) {

            String8 result(kDeadlockedString);

            write(fd, result.string(), result.size());
@@ -1260,7 +1253,7 @@ status_t AudioPolicyService::AudioCommandThread::dump(int fd) 
    result.append(buffer);

    write(fd, result.string(), result.size());



    bool locked = tryLock(mLock);

    bool locked = dumpTryLock(mLock);

    if (!locked) {

        String8 result2(kCmdDeadlockedString);

        write(fd, result2.string(), result2.size());

services/soundtrigger/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ cc_library_shared { 
        "libhardware",

        "libsoundtrigger",

        "libaudioclient",

        "libaudioutils",

        "libmediautils",



        "libhwbinder",

services/soundtrigger/SoundTriggerHwService.cpp

+6 −13
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ 
#include <sys/types.h>

#include <pthread.h>



#include <audio_utils/clock.h>

#include <system/sound_trigger.h>

#include <cutils/atomic.h>

#include <cutils/properties.h>
@@ -146,20 +147,12 @@ status_t SoundTriggerHwService::setCaptureState(bool active) 
}





static const int kDumpLockRetries = 50;

static const int kDumpLockSleep = 60000;

static const int kDumpLockTimeoutNs = 1 * NANOS_PER_SECOND;



static bool tryLock(Mutex& mutex)

static bool dumpTryLock(Mutex& mutex)

{

    bool locked = false;

    for (int i = 0; i < kDumpLockRetries; ++i) {

        if (mutex.tryLock() == NO_ERROR) {

            locked = true;

            break;

        }

        usleep(kDumpLockSleep);

    }

    return locked;

    status_t err = mutex.timedLock(kDumpLockTimeoutNs);

    return err == NO_ERROR;

}



status_t SoundTriggerHwService::dump(int fd, const Vector<String16>& args __unused) {
@@ -168,7 +161,7 @@ status_t SoundTriggerHwService::dump(int fd, const Vector<String16>& args __unus 
        result.appendFormat("Permission Denial: can't dump SoundTriggerHwService");

        write(fd, result.string(), result.size());

    } else {

        bool locked = tryLock(mServiceLock);

        bool locked = dumpTryLock(mServiceLock);

        // failed to lock - SoundTriggerHwService is probably deadlocked

        if (!locked) {

            result.append("SoundTriggerHwService may be deadlocked\n");