Merge "Enforce basic thread safety in Audio Policy Service" into rvc-dev
diff --git a/services/audiopolicy/service/Android.mk b/services/audiopolicy/service/Android.mk
index 94e4811..680b077 100644
--- a/services/audiopolicy/service/Android.mk
+++ b/services/audiopolicy/service/Android.mk
@@ -44,7 +44,7 @@
LOCAL_MODULE:= libaudiopolicyservice
LOCAL_CFLAGS += -fvisibility=hidden
-LOCAL_CFLAGS += -Wall -Werror
+LOCAL_CFLAGS += -Wall -Werror -Wthread-safety
include $(BUILD_SHARED_LIBRARY)
diff --git a/services/audiopolicy/service/AudioPolicyEffects.cpp b/services/audiopolicy/service/AudioPolicyEffects.cpp
index 738a279..1ec0c5e 100644
--- a/services/audiopolicy/service/AudioPolicyEffects.cpp
+++ b/services/audiopolicy/service/AudioPolicyEffects.cpp
@@ -928,7 +928,10 @@
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;
}
diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp
index a15970a..23de08b 100644
--- a/services/audiopolicy/service/AudioPolicyService.cpp
+++ b/services/audiopolicy/service/AudioPolicyService.cpp
@@ -58,8 +58,6 @@
AudioPolicyService::AudioPolicyService()
: BnAudioPolicyService(),
- mpAudioPolicyDev(NULL),
- mpAudioPolicy(NULL),
mAudioPolicyManager(NULL),
mAudioPolicyClient(NULL),
mPhoneState(AUDIO_MODE_INVALID),
@@ -78,21 +76,19 @@
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 @@
mAudioPolicyEffects.clear();
mUidPolicy->unregisterSelf();
- mUidPolicy.clear();
-
mSensorPrivacyPolicy->unregisterSelf();
+
+ mUidPolicy.clear();
mSensorPrivacyPolicy.clear();
}
@@ -172,20 +168,20 @@
// 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 @@
IPCThreadState::self()->getCallingPid());
}
-static bool dumpTryLock(Mutex& mutex)
+static bool dumpTryLock(Mutex& mutex) ACQUIRE(mutex) NO_THREAD_SAFETY_ANALYSIS
{
- status_t err = mutex.timedLock(kDumpLockTimeoutNs);
- return err == NO_ERROR;
+ return mutex.timedLock(kDumpLockTimeoutNs) == NO_ERROR;
+}
+
+static void dumpReleaseLock(Mutex& mutex, bool locked) RELEASE(mutex) NO_THREAD_SAFETY_ANALYSIS
+{
+ if (locked) mutex.unlock();
}
status_t AudioPolicyService::dumpInternals(int fd)
@@ -564,7 +564,7 @@
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
@@ -703,7 +703,7 @@
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());
@@ -720,7 +720,7 @@
mPackageManager.dump(fd);
- if (locked) mLock.unlock();
+ dumpReleaseLock(mLock, locked);
}
return NO_ERROR;
}
@@ -839,8 +839,16 @@
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) {
@@ -860,8 +868,16 @@
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) {
@@ -881,11 +897,15 @@
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) {
@@ -1402,7 +1422,7 @@
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());
@@ -1425,7 +1445,7 @@
write(fd, result.string(), result.size());
- if (locked) mLock.unlock();
+ dumpReleaseLock(mLock, locked);
return NO_ERROR;
}
diff --git a/services/audiopolicy/service/AudioPolicyService.h b/services/audiopolicy/service/AudioPolicyService.h
index f77a481..869a963 100644
--- a/services/audiopolicy/service/AudioPolicyService.h
+++ b/services/audiopolicy/service/AudioPolicyService.h
@@ -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 @@
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 @@
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 @@
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:
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 @@
// 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