gugelfrei
/
android_frameworks_av
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Enforce basic thread safety in Audio Policy Service

Browse Source 
Enable clang thread safety analysis. Convert comments on TS
into directives. Fix missing lock acquisitions. Add missing
default initializers for the fields of UidPolicy and
SensorPrivacyPolicy

Also remove unused fields mpAudioPolicyDev and mpAudioPolicy
from AudioPolicyService.

TODO: Consider protecting pointers to command threads,
APM instance and the client, and system usages with a separate
lock.

Bug: 70398235
Bug: 155336464
Test: make, flash, and test audio on device
      execute "adb shell cmd media.audio_policy" commands
Change-Id: I47b132c8b5c977812fad5c89fa57882a37779c18
Merged-In: I47b132c8b5c977812fad5c89fa57882a37779c18
gugelfrei
Mikhail Naganov 4 years ago
parent 929c364533
commit 12b716ce09
4 changed files with 80 additions and 55 deletions
Show Stats Download Patch File Download Diff File

2
services/audiopolicy/service/Android.mk
Unescape View File

@ -44,7 +44,7 @@ LOCAL_STATIC_LIBRARIES := \
LOCAL_MODULE:= libaudiopolicyservice
LOCAL_CFLAGS += -fvisibility=hidden
LOCAL_CFLAGS += -Wall -Werror
LOCAL_CFLAGS += -Wall -Werror -Wthread-safety
include $(BUILD_SHARED_LIBRARY)

5
services/audiopolicy/service/AudioPolicyEffects.cpp
Unescape View File

@ -928,7 +928,10 @@ status_t AudioPolicyEffects::loadAudioEffectXmlConfig() {
loadProcessingChain(result.parsedConfig->preprocess, mInputSources);
loadProcessingChain(result.parsedConfig->postprocess, mOutputStreams);
loadDeviceProcessingChain(result.parsedConfig->deviceprocess, mDeviceEffects);
{
Mutex::Autolock _l(mLock);
loadDeviceProcessingChain(result.parsedConfig->deviceprocess, mDeviceEffects);
}
// Casting from ssize_t to status_t is probably safe, there should not be more than 2^31 errors
return result.nbSkippedElement;
}

86
services/audiopolicy/service/AudioPolicyService.cpp
Unescape View File

@ -58,8 +58,6 @@ static const String16 sManageAudioPolicyPermission("android.permission.MANAGE_AU
AudioPolicyService::AudioPolicyService()
: BnAudioPolicyService(),
mpAudioPolicyDev(NULL),
mpAudioPolicy(NULL),
mAudioPolicyManager(NULL),
mAudioPolicyClient(NULL),
mPhoneState(AUDIO_MODE_INVALID),
@ -78,21 +76,19 @@ void AudioPolicyService::onFirstRef()
mAudioPolicyClient = new AudioPolicyClient(this);
mAudioPolicyManager = createAudioPolicyManager(mAudioPolicyClient);
mSupportedSystemUsages = std::vector<audio_usage_t> {};
}
// load audio processing modules
sp<AudioPolicyEffects>audioPolicyEffects = new AudioPolicyEffects();
sp<AudioPolicyEffects> audioPolicyEffects = new AudioPolicyEffects();
sp<UidPolicy> uidPolicy = new UidPolicy(this);
sp<SensorPrivacyPolicy> sensorPrivacyPolicy = new SensorPrivacyPolicy(this);
{
Mutex::Autolock _l(mLock);
mAudioPolicyEffects = audioPolicyEffects;
mUidPolicy = uidPolicy;
mSensorPrivacyPolicy = sensorPrivacyPolicy;
}
mUidPolicy = new UidPolicy(this);
mUidPolicy->registerSelf();
mSensorPrivacyPolicy = new SensorPrivacyPolicy(this);
mSensorPrivacyPolicy->registerSelf();
uidPolicy->registerSelf();
sensorPrivacyPolicy->registerSelf();
}
AudioPolicyService::~AudioPolicyService()
@ -107,9 +103,9 @@ AudioPolicyService::~AudioPolicyService()
mAudioPolicyEffects.clear();
mUidPolicy->unregisterSelf();
mUidPolicy.clear();
mSensorPrivacyPolicy->unregisterSelf();
mUidPolicy.clear();
mSensorPrivacyPolicy.clear();
}
@ -172,20 +168,20 @@ void AudioPolicyService::setAudioVolumeGroupCallbacksEnabled(bool enabled)
// removeNotificationClient() is called when the client process dies.
void AudioPolicyService::removeNotificationClient(uid_t uid, pid_t pid)
{
bool hasSameUid = false;
{
Mutex::Autolock _l(mNotificationClientsLock);
int64_t token = ((int64_t)uid<<32) | pid;
mNotificationClients.removeItem(token);
}
{
Mutex::Autolock _l(mLock);
bool hasSameUid = false;
for (size_t i = 0; i < mNotificationClients.size(); i++) {
if (mNotificationClients.valueAt(i)->uid() == uid) {
hasSameUid = true;
break;
}
}
}
{
Mutex::Autolock _l(mLock);
if (mAudioPolicyManager && !hasSameUid) {
// called from binder death notification: no need to clear caller identity
mAudioPolicyManager->releaseResourcesForUid(uid);
@ -381,10 +377,14 @@ void AudioPolicyService::binderDied(const wp<IBinder>& who) {
IPCThreadState::self()->getCallingPid());
}
static bool dumpTryLock(Mutex& mutex)
static bool dumpTryLock(Mutex& mutex) ACQUIRE(mutex) NO_THREAD_SAFETY_ANALYSIS
{
return mutex.timedLock(kDumpLockTimeoutNs) == NO_ERROR;
}
static void dumpReleaseLock(Mutex& mutex, bool locked) RELEASE(mutex) NO_THREAD_SAFETY_ANALYSIS
{
status_t err = mutex.timedLock(kDumpLockTimeoutNs);
return err == NO_ERROR;
if (locked) mutex.unlock();
}
status_t AudioPolicyService::dumpInternals(int fd)
@ -564,7 +564,7 @@ void AudioPolicyService::updateUidStates_l()
bool isTopOrLatestSensitive = topSensitiveActive == nullptr ?
false : current->uid == topSensitiveActive->uid;
auto canCaptureIfInCallOrCommunication = [&](const auto &recordClient) {
auto canCaptureIfInCallOrCommunication = [&](const auto &recordClient) REQUIRES(mLock) {
bool canCaptureCall = recordClient->canCaptureOutput;
bool canCaptureCommunication = recordClient->canCaptureOutput
|| recordClient->uid == mPhoneStateOwnerUid
@ -702,7 +702,7 @@ status_t AudioPolicyService::dump(int fd, const Vector<String16>& args __unused)
if (!dumpAllowed()) {
dumpPermissionDenial(fd);
} else {
bool locked = dumpTryLock(mLock);
const bool locked = dumpTryLock(mLock);
if (!locked) {
String8 result(kDeadlockedString);
write(fd, result.string(), result.size());
@ -719,7 +719,7 @@ status_t AudioPolicyService::dump(int fd, const Vector<String16>& args __unused)
mPackageManager.dump(fd);
if (locked) mLock.unlock();
dumpReleaseLock(mLock, locked);
}
return NO_ERROR;
}
@ -838,8 +838,16 @@ status_t AudioPolicyService::handleSetUidState(Vector<String16>& args, int err)
return BAD_VALUE;
}
mUidPolicy->addOverrideUid(uid, active);
return NO_ERROR;
sp<UidPolicy> uidPolicy;
{
Mutex::Autolock _l(mLock);
uidPolicy = mUidPolicy;
}
if (uidPolicy) {
uidPolicy->addOverrideUid(uid, active);
return NO_ERROR;
}
return NO_INIT;
}
status_t AudioPolicyService::handleResetUidState(Vector<String16>& args, int err) {
@ -859,8 +867,16 @@ status_t AudioPolicyService::handleResetUidState(Vector<String16>& args, int err
return BAD_VALUE;
}
mUidPolicy->removeOverrideUid(uid);
return NO_ERROR;
sp<UidPolicy> uidPolicy;
{
Mutex::Autolock _l(mLock);
uidPolicy = mUidPolicy;
}
if (uidPolicy) {
uidPolicy->removeOverrideUid(uid);
return NO_ERROR;
}
return NO_INIT;
}
status_t AudioPolicyService::handleGetUidState(Vector<String16>& args, int out, int err) {
@ -880,11 +896,15 @@ status_t AudioPolicyService::handleGetUidState(Vector<String16>& args, int out,
return BAD_VALUE;
}
if (mUidPolicy->isUidActive(uid)) {
return dprintf(out, "active\n");
} else {
return dprintf(out, "idle\n");
sp<UidPolicy> uidPolicy;
{
Mutex::Autolock _l(mLock);
uidPolicy = mUidPolicy;
}
if (uidPolicy) {
return dprintf(out, uidPolicy->isUidActive(uid) ? "active\n" : "idle\n");
}
return NO_INIT;
}
status_t AudioPolicyService::printHelp(int out) {
@ -1401,7 +1421,7 @@ status_t AudioPolicyService::AudioCommandThread::dump(int fd)
result.append(buffer);
write(fd, result.string(), result.size());
bool locked = dumpTryLock(mLock);
const bool locked = dumpTryLock(mLock);
if (!locked) {
String8 result2(kCmdDeadlockedString);
write(fd, result2.string(), result2.size());
@ -1424,7 +1444,7 @@ status_t AudioPolicyService::AudioCommandThread::dump(int fd)
write(fd, result.string(), result.size());
if (locked) mLock.unlock();
dumpReleaseLock(mLock, locked);
return NO_ERROR;
}

42
services/audiopolicy/service/AudioPolicyService.h
Unescape View File

@ -17,6 +17,7 @@
#ifndef ANDROID_AUDIOPOLICYSERVICE_H
#define ANDROID_AUDIOPOLICYSERVICE_H
#include <android-base/thread_annotations.h>
#include <cutils/misc.h>
#include <cutils/config_utils.h>
#include <cutils/compiler.h>
@ -330,13 +331,13 @@ private:
AudioPolicyService() ANDROID_API;
virtual ~AudioPolicyService();
status_t dumpInternals(int fd);
status_t dumpInternals(int fd) REQUIRES(mLock);
// Handles binder shell commands
virtual status_t shellCommand(int in, int out, int err, Vector<String16>& args);
// Sets whether the given UID records only silence
virtual void setAppState_l(audio_port_handle_t portId, app_state_t state);
virtual void setAppState_l(audio_port_handle_t portId, app_state_t state) REQUIRES(mLock);
// Overrides the UID state as if it is idle
status_t handleSetUidState(Vector<String16>& args, int err);
@ -361,9 +362,9 @@ private:
status_t validateUsage(audio_usage_t usage, pid_t pid, uid_t uid);
void updateUidStates();
void updateUidStates_l();
void updateUidStates_l() REQUIRES(mLock);
void silenceAllRecordings_l();
void silenceAllRecordings_l() REQUIRES(mLock);
static bool isVirtualSource(audio_source_t source);
@ -420,13 +421,13 @@ private:
wp<AudioPolicyService> mService;
Mutex mLock;
ActivityManager mAm;
bool mObserverRegistered;
bool mObserverRegistered = false;
std::unordered_map<uid_t, std::pair<bool, int>> mOverrideUids;
std::unordered_map<uid_t, std::pair<bool, int>> mCachedUids;
uid_t mAssistantUid;
uid_t mAssistantUid = -1;
std::vector<uid_t> mA11yUids;
uid_t mCurrentImeUid;
bool mRttEnabled;
uid_t mCurrentImeUid = -1;
bool mRttEnabled = false;
};
// If sensor privacy is enabled then all apps, including those that are active, should be
@ -447,7 +448,7 @@ private:
private:
wp<AudioPolicyService> mService;
std::atomic_bool mSensorPrivacyEnabled;
std::atomic_bool mSensorPrivacyEnabled = false;
};
// Thread used to send audio config commands to audio flinger
@ -880,26 +881,27 @@ private:
// and possibly back in to audio policy service and acquire mEffectsLock.
sp<AudioCommandThread> mAudioCommandThread; // audio commands thread
sp<AudioCommandThread> mOutputCommandThread; // process stop and release output
struct audio_policy_device *mpAudioPolicyDev;
struct audio_policy *mpAudioPolicy;
AudioPolicyInterface *mAudioPolicyManager;
AudioPolicyClient *mAudioPolicyClient;
std::vector<audio_usage_t> mSupportedSystemUsages;
DefaultKeyedVector< int64_t, sp<NotificationClient> > mNotificationClients;
Mutex mNotificationClientsLock; // protects mNotificationClients
Mutex mNotificationClientsLock;
DefaultKeyedVector<int64_t, sp<NotificationClient>> mNotificationClients
GUARDED_BY(mNotificationClientsLock);
// Manage all effects configured in audio_effects.conf
// never hold AudioPolicyService::mLock when calling AudioPolicyEffects methods as
// those can call back into AudioPolicyService methods and try to acquire the mutex
sp<AudioPolicyEffects> mAudioPolicyEffects;
audio_mode_t mPhoneState;
uid_t mPhoneStateOwnerUid;
sp<AudioPolicyEffects> mAudioPolicyEffects GUARDED_BY(mLock);
audio_mode_t mPhoneState GUARDED_BY(mLock);
uid_t mPhoneStateOwnerUid GUARDED_BY(mLock);
sp<UidPolicy> mUidPolicy;
sp<SensorPrivacyPolicy> mSensorPrivacyPolicy;
sp<UidPolicy> mUidPolicy GUARDED_BY(mLock);
sp<SensorPrivacyPolicy> mSensorPrivacyPolicy GUARDED_BY(mLock);
DefaultKeyedVector< audio_port_handle_t, sp<AudioRecordClient> > mAudioRecordClients;
DefaultKeyedVector< audio_port_handle_t, sp<AudioPlaybackClient> > mAudioPlaybackClients;
DefaultKeyedVector<audio_port_handle_t, sp<AudioRecordClient>> mAudioRecordClients
GUARDED_BY(mLock);
DefaultKeyedVector<audio_port_handle_t, sp<AudioPlaybackClient>> mAudioPlaybackClients
GUARDED_BY(mLock);
MediaPackageManager mPackageManager; // To check allowPlaybackCapture

Write Preview
Loading…
Cancel
Save