Merge "Enforce basic thread safety in Audio Policy Service" into rvc-dev (6ffc5162) · Commits · e / os / android_frameworks_av

services/audiopolicy/service/Android.mk

+1 −1

Original line number	Diff line number	Diff line
		@@ -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)

services/audiopolicy/service/AudioPolicyEffects.cpp

+4 −1

Original line number	Diff line number	Diff line
		@@ -928,7 +928,10 @@ status_t AudioPolicyEffects::loadAudioEffectXmlConfig() {

		loadProcessingChain(result.parsedConfig->preprocess, mInputSources);
		loadProcessingChain(result.parsedConfig->postprocess, mOutputStreams);
		{
		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;
		}

services/audiopolicy/service/AudioPolicyService.cpp

+53 −33

Original line number	Diff line number	Diff line
		@@ -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<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
		{
		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 @@ 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
		@@ -703,7 +703,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());
		@@ -720,7 +720,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;
		}
		@@ -839,9 +839,17 @@ status_t AudioPolicyService::handleSetUidState(Vector<String16>& args, int err)
		return BAD_VALUE;
		}

		mUidPolicy->addOverrideUid(uid, active);
		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) {
		// Valid arg.size() is 2 or 4, args.size() is 4 with --user option.
		@@ -860,9 +868,17 @@ status_t AudioPolicyService::handleResetUidState(Vector<String16>& args, int err
		return BAD_VALUE;
		}

		mUidPolicy->removeOverrideUid(uid);
		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) {
		// Valid arg.size() is 2 or 4, args.size() is 4 with --user option.
		@@ -881,11 +897,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) {
		@@ -1402,7 +1422,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());
		@@ -1425,7 +1445,7 @@ status_t AudioPolicyService::AudioCommandThread::dump(int fd)

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

		if (locked) mLock.unlock();
		dumpReleaseLock(mLock, locked);

		return NO_ERROR;
		}

services/audiopolicy/service/AudioPolicyService.h

+22 −20

Original line number	Diff line number	Diff line
		@@ -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