From 42896a0774caae74ea8c982417ecbefb21913bc8 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Fri, 27 Sep 2019 15:40:33 -0700 Subject: [PATCH] media utils: dump audio HAL service before restarting audioserver Add request to create tombstones of audio HAL servers before restarting audioserver process when the watchdog triggers. Add audio device factory HAL interface API to retrieve HAL process pids when possible (on debug builds). Add AudioFlinger service API to set audio HAL process pids from JAVA AudioService. Bug: 141528385 Test: Force watchdog and verify tombstone creation Change-Id: I68c1e8fb4db23e5952ad0c93d7d0b9d121b8ec18 --- media/libaudioclient/AudioSystem.cpp | 6 +++ media/libaudioclient/IAudioFlinger.cpp | 44 ++++++++++++++++- .../include/media/AudioSystem.h | 6 +++ .../include/media/IAudioFlinger.h | 2 + .../impl/DevicesFactoryHalHidl.cpp | 27 ++++++++++ .../libaudiohal/impl/DevicesFactoryHalHidl.h | 3 ++ .../impl/DevicesFactoryHalHybrid.cpp | 8 +++ .../impl/DevicesFactoryHalHybrid.h | 2 + .../libaudiohal/impl/DevicesFactoryHalLocal.h | 4 ++ .../audiohal/DevicesFactoryHalInterface.h | 3 ++ media/utils/Android.bp | 4 ++ media/utils/TimeCheck.cpp | 49 +++++++++++++++++++ media/utils/include/mediautils/TimeCheck.h | 5 +- services/audioflinger/AudioFlinger.cpp | 9 ++++ services/audioflinger/AudioFlinger.h | 2 + 15 files changed, 172 insertions(+), 2 deletions(-) diff --git a/media/libaudioclient/AudioSystem.cpp b/media/libaudioclient/AudioSystem.cpp index 20ca35cfb8..54184e08e0 100644 --- a/media/libaudioclient/AudioSystem.cpp +++ b/media/libaudioclient/AudioSystem.cpp @@ -1392,6 +1392,12 @@ status_t AudioSystem::getMicrophones(std::vector *microph return af->getMicrophones(microphones); } +status_t AudioSystem::setAudioHalPids(const std::vector& pids) { + const sp& af = AudioSystem::get_audio_flinger(); + if (af == nullptr) return PERMISSION_DENIED; + return af->setAudioHalPids(pids); +} + status_t AudioSystem::getSurroundFormats(unsigned int *numSurroundFormats, audio_format_t *surroundFormats, bool *surroundFormatsEnabled, diff --git a/media/libaudioclient/IAudioFlinger.cpp b/media/libaudioclient/IAudioFlinger.cpp index 6e9a7cf06d..04ef3dd902 100644 --- a/media/libaudioclient/IAudioFlinger.cpp +++ b/media/libaudioclient/IAudioFlinger.cpp @@ -90,10 +90,12 @@ enum { SET_MASTER_BALANCE, GET_MASTER_BALANCE, SET_EFFECT_SUSPENDED, + SET_AUDIO_HAL_PIDS }; #define MAX_ITEMS_PER_LIST 1024 + class BpAudioFlinger : public BpInterface { public: @@ -900,6 +902,20 @@ public: status = reply.readParcelableVector(microphones); return status; } + virtual status_t setAudioHalPids(const std::vector& pids) + { + Parcel data, reply; + data.writeInterfaceToken(IAudioFlinger::getInterfaceDescriptor()); + data.writeInt32(pids.size()); + for (auto pid : pids) { + data.writeInt32(pid); + } + status_t status = remote()->transact(SET_AUDIO_HAL_PIDS, data, &reply); + if (status != NO_ERROR) { + return status; + } + return static_cast (reply.readInt32()); + } }; IMPLEMENT_META_INTERFACE(AudioFlinger, "android.media.IAudioFlinger"); @@ -955,7 +971,8 @@ status_t BnAudioFlinger::onTransact( case SET_MODE: case SET_MIC_MUTE: case SET_LOW_RAM_DEVICE: - case SYSTEM_READY: { + case SYSTEM_READY: + case SET_AUDIO_HAL_PIDS: { if (!isServiceUid(IPCThreadState::self()->getCallingUid())) { ALOGW("%s: transaction %d received from PID %d unauthorized UID %d", __func__, code, IPCThreadState::self()->getCallingPid(), @@ -1544,6 +1561,31 @@ status_t BnAudioFlinger::onTransact( } return NO_ERROR; } + case SET_AUDIO_HAL_PIDS: { + CHECK_INTERFACE(IAudioFlinger, data, reply); + std::vector pids; + int32_t size; + status_t status = data.readInt32(&size); + if (status != NO_ERROR) { + return status; + } + if (size < 0) { + return BAD_VALUE; + } + if (size > MAX_ITEMS_PER_LIST) { + size = MAX_ITEMS_PER_LIST; + } + for (int32_t i = 0; i < size; i++) { + int32_t pid; + status = data.readInt32(&pid); + if (status != NO_ERROR) { + return status; + } + pids.push_back(pid); + } + reply->writeInt32(setAudioHalPids(pids)); + return NO_ERROR; + } default: return BBinder::onTransact(code, data, reply, flags); } diff --git a/media/libaudioclient/include/media/AudioSystem.h b/media/libaudioclient/include/media/AudioSystem.h index 12da0de26e..d7266b4b76 100644 --- a/media/libaudioclient/include/media/AudioSystem.h +++ b/media/libaudioclient/include/media/AudioSystem.h @@ -399,6 +399,12 @@ public: static bool isCallScreenModeSupported(); + /** + * Send audio HAL server process pids to native audioserver process for use + * when generating audio HAL servers tombstones + */ + static status_t setAudioHalPids(const std::vector& pids); + // ---------------------------------------------------------------------------- class AudioVolumeGroupCallback : public RefBase diff --git a/media/libaudioclient/include/media/IAudioFlinger.h b/media/libaudioclient/include/media/IAudioFlinger.h index 0a65857580..308e9d303c 100644 --- a/media/libaudioclient/include/media/IAudioFlinger.h +++ b/media/libaudioclient/include/media/IAudioFlinger.h @@ -523,6 +523,8 @@ public: /* List available microphones and their characteristics */ virtual status_t getMicrophones(std::vector *microphones) = 0; + + virtual status_t setAudioHalPids(const std::vector& pids) = 0; }; diff --git a/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp b/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp index 1335a0c36c..c30da3c310 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp +++ b/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp @@ -20,6 +20,7 @@ #define LOG_TAG "DevicesFactoryHalHidl" //#define LOG_NDEBUG 0 +#include "android/hidl/manager/1.0/IServiceManager.h" #include PATH(android/hardware/audio/FILE_VERSION/IDevice.h) #include #include @@ -28,6 +29,8 @@ #include "DeviceHalHidl.h" #include "DevicesFactoryHalHidl.h" +#include + using ::android::hardware::audio::CPP_VERSION::IDevice; using ::android::hardware::audio::CPP_VERSION::Result; using ::android::hardware::Return; @@ -108,5 +111,29 @@ status_t DevicesFactoryHalHidl::openDevice(const char *name, sp *pids) { + std::set pidsSet; + + for (const auto& factory : mDeviceFactories) { + using ::android::hidl::base::V1_0::DebugInfo; + using android::hidl::manager::V1_0::IServiceManager; + + DebugInfo debugInfo; + auto ret = factory->getDebugInfo([&] (const auto &info) { + debugInfo = info; + }); + if (!ret.isOk()) { + return INVALID_OPERATION; + } + if (debugInfo.pid == (int)IServiceManager::PidConstant::NO_PID) { + continue; + } + pidsSet.insert(debugInfo.pid); + } + + *pids = {pidsSet.begin(), pidsSet.end()}; + return NO_ERROR; +} + } // namespace CPP_VERSION } // namespace android diff --git a/media/libaudiohal/impl/DevicesFactoryHalHidl.h b/media/libaudiohal/impl/DevicesFactoryHalHidl.h index 8775e7bf81..52185c8641 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHidl.h +++ b/media/libaudiohal/impl/DevicesFactoryHalHidl.h @@ -37,6 +37,9 @@ class DevicesFactoryHalHidl : public DevicesFactoryHalInterface // Opens a device with the specified name. To close the device, it is // necessary to release references to the returned object. virtual status_t openDevice(const char *name, sp *device); + + status_t getHalPids(std::vector *pids) override; + private: std::vector> mDeviceFactories; diff --git a/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp b/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp index 0e1f1bb9c1..a5aef1b517 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp +++ b/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp @@ -37,6 +37,14 @@ status_t DevicesFactoryHalHybrid::openDevice(const char *name, spopenDevice(name, device); } + +status_t DevicesFactoryHalHybrid::getHalPids(std::vector *pids) { + if (mHidlFactory != 0) { + return mHidlFactory->getHalPids(pids); + } + return INVALID_OPERATION; +} + } // namespace CPP_VERSION template <> diff --git a/media/libaudiohal/impl/DevicesFactoryHalHybrid.h b/media/libaudiohal/impl/DevicesFactoryHalHybrid.h index 545bb70a0a..2189b36f87 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHybrid.h +++ b/media/libaudiohal/impl/DevicesFactoryHalHybrid.h @@ -36,6 +36,8 @@ class DevicesFactoryHalHybrid : public DevicesFactoryHalInterface // necessary to release references to the returned object. virtual status_t openDevice(const char *name, sp *device); + status_t getHalPids(std::vector *pids) override; + private: sp mLocalFactory; sp mHidlFactory; diff --git a/media/libaudiohal/impl/DevicesFactoryHalLocal.h b/media/libaudiohal/impl/DevicesFactoryHalLocal.h index 5d108dd443..2b011f47b8 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalLocal.h +++ b/media/libaudiohal/impl/DevicesFactoryHalLocal.h @@ -33,6 +33,10 @@ class DevicesFactoryHalLocal : public DevicesFactoryHalInterface // necessary to release references to the returned object. virtual status_t openDevice(const char *name, sp *device); + status_t getHalPids(std::vector *pids __unused) override { + return INVALID_OPERATION; + } + private: friend class DevicesFactoryHalHybrid; diff --git a/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h b/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h index 14af384ac2..e9ac1ce34d 100644 --- a/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h +++ b/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h @@ -20,6 +20,7 @@ #include #include #include +#include namespace android { @@ -30,6 +31,8 @@ class DevicesFactoryHalInterface : public RefBase // necessary to release references to the returned object. virtual status_t openDevice(const char *name, sp *device) = 0; + virtual status_t getHalPids(std::vector *pids) = 0; + static sp create(); protected: diff --git a/media/utils/Android.bp b/media/utils/Android.bp index 8a9039c54f..5047b19646 100644 --- a/media/utils/Android.bp +++ b/media/utils/Android.bp @@ -51,6 +51,10 @@ cc_library { "libmedia_headers", ], + include_dirs: [ + // For DEBUGGER_SIGNAL + "system/core/debuggerd/include", + ], local_include_dirs: ["include"], export_include_dirs: ["include"], } diff --git a/media/utils/TimeCheck.cpp b/media/utils/TimeCheck.cpp index 96f78023a3..4a3e470e05 100644 --- a/media/utils/TimeCheck.cpp +++ b/media/utils/TimeCheck.cpp @@ -14,13 +14,50 @@ * limitations under the License. */ +#define LOG_TAG "TimeCheck" #include #include #include +#include "debuggerd/handler.h" namespace android { +// Audio HAL server pids vector used to generate audio HAL processes tombstone +// when audioserver watchdog triggers. +// We use a lockless storage to avoid potential deadlocks in the context of watchdog +// trigger. +// Protection again simultaneous writes is not needed given one update takes place +// during AudioFlinger construction and other comes necessarily later once the IAudioFlinger +// interface is available. +// The use of an atomic index just guaranties that current vector is fully initialized +// when read. +/* static */ +void TimeCheck::accessAudioHalPids(std::vector* pids, bool update) { + static constexpr int kNumAudioHalPidsVectors = 3; + static std::vector audioHalPids[kNumAudioHalPidsVectors]; + static std::atomic curAudioHalPids = 0; + + if (update) { + audioHalPids[(curAudioHalPids + 1) % kNumAudioHalPidsVectors] = *pids; + curAudioHalPids++; + } else { + *pids = audioHalPids[curAudioHalPids]; + } +} + +/* static */ +void TimeCheck::setAudioHalPids(const std::vector& pids) { + accessAudioHalPids(&(const_cast&>(pids)), true); +} + +/* static */ +std::vector TimeCheck::getAudioHalPids() { + std::vector pids; + accessAudioHalPids(&pids, false); + return pids; +} + /* static */ sp TimeCheck::getTimeCheckThread() { @@ -83,6 +120,18 @@ bool TimeCheck::TimeCheckThread::threadLoop() status = mCond.waitRelative(mMutex, waitTimeNs); } if (status != NO_ERROR) { + // Generate audio HAL processes tombstones and allow time to complete + // before forcing restart + std::vector pids = getAudioHalPids(); + if (pids.size() != 0) { + for (const auto& pid : pids) { + ALOGI("requesting tombstone for pid: %d", pid); + sigqueue(pid, DEBUGGER_SIGNAL, {.sival_int = 0}); + } + sleep(1); + } else { + ALOGI("No HAL process pid available, skipping tombstones"); + } LOG_EVENT_STRING(LOGTAG_AUDIO_BINDER_TIMEOUT, tag); LOG_ALWAYS_FATAL("TimeCheck timeout for %s", tag); } diff --git a/media/utils/include/mediautils/TimeCheck.h b/media/utils/include/mediautils/TimeCheck.h index 6c5f656ebc..5ba6d7cdf8 100644 --- a/media/utils/include/mediautils/TimeCheck.h +++ b/media/utils/include/mediautils/TimeCheck.h @@ -20,7 +20,7 @@ #include #include - +#include namespace android { @@ -35,6 +35,8 @@ public: TimeCheck(const char *tag, uint32_t timeoutMs = kDefaultTimeOutMs); ~TimeCheck(); + static void setAudioHalPids(const std::vector& pids); + static std::vector getAudioHalPids(); private: @@ -63,6 +65,7 @@ private: }; static sp getTimeCheckThread(); + static void accessAudioHalPids(std::vector* pids, bool update); const nsecs_t mEndTimeNs; }; diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index 9d80425f60..fce29b5fb2 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -70,6 +70,7 @@ #include #include #include +#include #include //#define BUFLOG_NDEBUG 0 @@ -193,6 +194,9 @@ AudioFlinger::AudioFlinger() mEffectsFactoryHal = EffectsFactoryHalInterface::create(); mMediaLogNotifier->run("MediaLogNotifier"); + std::vector halPids; + mDevicesFactoryHal->getHalPids(&halPids); + TimeCheck::setAudioHalPids(halPids); } void AudioFlinger::onFirstRef() @@ -218,6 +222,11 @@ void AudioFlinger::onFirstRef() gAudioFlinger = this; } +status_t AudioFlinger::setAudioHalPids(const std::vector& pids) { + TimeCheck::setAudioHalPids(pids); + return NO_ERROR; +} + AudioFlinger::~AudioFlinger() { while (!mRecordThreads.isEmpty()) { diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 65be06d0de..974bd6969c 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -278,6 +278,8 @@ public: virtual status_t getMicrophones(std::vector *microphones); + virtual status_t setAudioHalPids(const std::vector& pids); + virtual status_t onTransact( uint32_t code, const Parcel& data,