Merge "Enforce basic thread safety in Audio Policy Service" into rvc-dev am:... (3bb29463) · Commits · e / os / android_frameworks_av

services/audiopolicy/service/Android.mk

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -44,7 +44,7 @@ LOCAL_STATIC_LIBRARIES := \
	LOCAL_MODULE:= libaudiopolicyservice		LOCAL_MODULE:= libaudiopolicyservice

	LOCAL_CFLAGS += -fvisibility=hidden		LOCAL_CFLAGS += -fvisibility=hidden
	LOCAL_CFLAGS += -Wall -Werror		LOCAL_CFLAGS += -Wall -Werror -Wthread-safety

	include $(BUILD_SHARED_LIBRARY)		include $(BUILD_SHARED_LIBRARY)

services/audiopolicy/service/AudioPolicyEffects.cpp

+4 −1

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

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

services/audiopolicy/service/AudioPolicyService.cpp

+53 −33

Original line number	Original line	Diff line number	Diff line
	@@ -58,8 +58,6 @@ static const String16 sManageAudioPolicyPermission("android.permission.MANAGE_AU

	AudioPolicyService::AudioPolicyService()		AudioPolicyService::AudioPolicyService()
	: BnAudioPolicyService(),		: BnAudioPolicyService(),
	mpAudioPolicyDev(NULL),
	mpAudioPolicy(NULL),
	mAudioPolicyManager(NULL),		mAudioPolicyManager(NULL),
	mAudioPolicyClient(NULL),		mAudioPolicyClient(NULL),
	mPhoneState(AUDIO_MODE_INVALID),		mPhoneState(AUDIO_MODE_INVALID),
	@@ -78,21 +76,19 @@ void AudioPolicyService::onFirstRef()

	mAudioPolicyClient = new AudioPolicyClient(this);		mAudioPolicyClient = new AudioPolicyClient(this);
	mAudioPolicyManager = createAudioPolicyManager(mAudioPolicyClient);		mAudioPolicyManager = createAudioPolicyManager(mAudioPolicyClient);

	mSupportedSystemUsages = std::vector<audio_usage_t> {};
	}		}
	// load audio processing modules		// 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);		Mutex::Autolock _l(mLock);
	mAudioPolicyEffects = audioPolicyEffects;		mAudioPolicyEffects = audioPolicyEffects;
			mUidPolicy = uidPolicy;
			mSensorPrivacyPolicy = sensorPrivacyPolicy;
	}		}
			uidPolicy->registerSelf();
	mUidPolicy = new UidPolicy(this);		sensorPrivacyPolicy->registerSelf();
	mUidPolicy->registerSelf();

	mSensorPrivacyPolicy = new SensorPrivacyPolicy(this);
	mSensorPrivacyPolicy->registerSelf();
	}		}

	AudioPolicyService::~AudioPolicyService()		AudioPolicyService::~AudioPolicyService()
	@@ -107,9 +103,9 @@ AudioPolicyService::~AudioPolicyService()
	mAudioPolicyEffects.clear();		mAudioPolicyEffects.clear();

	mUidPolicy->unregisterSelf();		mUidPolicy->unregisterSelf();
	mUidPolicy.clear();

	mSensorPrivacyPolicy->unregisterSelf();		mSensorPrivacyPolicy->unregisterSelf();

			mUidPolicy.clear();
	mSensorPrivacyPolicy.clear();		mSensorPrivacyPolicy.clear();
	}		}

	@@ -172,20 +168,20 @@ void AudioPolicyService::setAudioVolumeGroupCallbacksEnabled(bool enabled)
	// removeNotificationClient() is called when the client process dies.		// removeNotificationClient() is called when the client process dies.
	void AudioPolicyService::removeNotificationClient(uid_t uid, pid_t pid)		void AudioPolicyService::removeNotificationClient(uid_t uid, pid_t pid)
	{		{
			bool hasSameUid = false;
	{		{
	Mutex::Autolock _l(mNotificationClientsLock);		Mutex::Autolock _l(mNotificationClientsLock);
	int64_t token = ((int64_t)uid<<32) \| pid;		int64_t token = ((int64_t)uid<<32) \| pid;
	mNotificationClients.removeItem(token);		mNotificationClients.removeItem(token);
	}
	{
	Mutex::Autolock _l(mLock);
	bool hasSameUid = false;
	for (size_t i = 0; i < mNotificationClients.size(); i++) {		for (size_t i = 0; i < mNotificationClients.size(); i++) {
	if (mNotificationClients.valueAt(i)->uid() == uid) {		if (mNotificationClients.valueAt(i)->uid() == uid) {
	hasSameUid = true;		hasSameUid = true;
	break;		break;
	}		}
	}		}
			}
			{
			Mutex::Autolock _l(mLock);
	if (mAudioPolicyManager && !hasSameUid) {		if (mAudioPolicyManager && !hasSameUid) {
	// called from binder death notification: no need to clear caller identity		// called from binder death notification: no need to clear caller identity
	mAudioPolicyManager->releaseResourcesForUid(uid);		mAudioPolicyManager->releaseResourcesForUid(uid);
	@@ -381,10 +377,14 @@ void AudioPolicyService::binderDied(const wp<IBinder>& who) {
	IPCThreadState::self()->getCallingPid());		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 mutex.timedLock(kDumpLockTimeoutNs) == NO_ERROR;
	return err == 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)		status_t AudioPolicyService::dumpInternals(int fd)
	@@ -564,7 +564,7 @@ void AudioPolicyService::updateUidStates_l()
	bool isTopOrLatestSensitive = topSensitiveActive == nullptr ?		bool isTopOrLatestSensitive = topSensitiveActive == nullptr ?
	false : current->uid == topSensitiveActive->uid;		false : current->uid == topSensitiveActive->uid;

	auto canCaptureIfInCallOrCommunication = [&](const auto &recordClient) {		auto canCaptureIfInCallOrCommunication = [&](const auto &recordClient) REQUIRES(mLock) {
	bool canCaptureCall = recordClient->canCaptureOutput;		bool canCaptureCall = recordClient->canCaptureOutput;
	bool canCaptureCommunication = recordClient->canCaptureOutput		bool canCaptureCommunication = recordClient->canCaptureOutput
	\|\| recordClient->uid == mPhoneStateOwnerUid		\|\| recordClient->uid == mPhoneStateOwnerUid
	@@ -703,7 +703,7 @@ status_t AudioPolicyService::dump(int fd, const Vector<String16>& args __unused)
	if (!dumpAllowed()) {		if (!dumpAllowed()) {
	dumpPermissionDenial(fd);		dumpPermissionDenial(fd);
	} else {		} else {
	bool locked = dumpTryLock(mLock);		const bool locked = dumpTryLock(mLock);
	if (!locked) {		if (!locked) {
	String8 result(kDeadlockedString);		String8 result(kDeadlockedString);
	write(fd, result.string(), result.size());		write(fd, result.string(), result.size());
	@@ -720,7 +720,7 @@ status_t AudioPolicyService::dump(int fd, const Vector<String16>& args __unused)

	mPackageManager.dump(fd);		mPackageManager.dump(fd);

	if (locked) mLock.unlock();		dumpReleaseLock(mLock, locked);
	}		}
	return NO_ERROR;		return NO_ERROR;
	}		}
	@@ -839,9 +839,17 @@ status_t AudioPolicyService::handleSetUidState(Vector<String16>& args, int err)
	return BAD_VALUE;		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_ERROR;
	}		}
			return NO_INIT;
			}

	status_t AudioPolicyService::handleResetUidState(Vector<String16>& args, int err) {		status_t AudioPolicyService::handleResetUidState(Vector<String16>& args, int err) {
	// Valid arg.size() is 2 or 4, args.size() is 4 with --user option.		// 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;		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_ERROR;
	}		}
			return NO_INIT;
			}

	status_t AudioPolicyService::handleGetUidState(Vector<String16>& args, int out, int err) {		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.		// 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;		return BAD_VALUE;
	}		}

	if (mUidPolicy->isUidActive(uid)) {		sp<UidPolicy> uidPolicy;
	return dprintf(out, "active\n");		{
	} else {		Mutex::Autolock _l(mLock);
	return dprintf(out, "idle\n");		uidPolicy = mUidPolicy;
			}
			if (uidPolicy) {
			return dprintf(out, uidPolicy->isUidActive(uid) ? "active\n" : "idle\n");
	}		}
			return NO_INIT;
	}		}

	status_t AudioPolicyService::printHelp(int out) {		status_t AudioPolicyService::printHelp(int out) {
	@@ -1402,7 +1422,7 @@ status_t AudioPolicyService::AudioCommandThread::dump(int fd)
	result.append(buffer);		result.append(buffer);
	write(fd, result.string(), result.size());		write(fd, result.string(), result.size());

	bool locked = dumpTryLock(mLock);		const bool locked = dumpTryLock(mLock);
	if (!locked) {		if (!locked) {
	String8 result2(kCmdDeadlockedString);		String8 result2(kCmdDeadlockedString);
	write(fd, result2.string(), result2.size());		write(fd, result2.string(), result2.size());
	@@ -1425,7 +1445,7 @@ status_t AudioPolicyService::AudioCommandThread::dump(int fd)

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

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

	return NO_ERROR;		return NO_ERROR;
	}		}

services/audiopolicy/service/AudioPolicyService.h

+22 −20

Original line number	Original line	Diff line number	Diff line
	@@ -17,6 +17,7 @@
	#ifndef ANDROID_AUDIOPOLICYSERVICE_H		#ifndef ANDROID_AUDIOPOLICYSERVICE_H
	#define ANDROID_AUDIOPOLICYSERVICE_H		#define ANDROID_AUDIOPOLICYSERVICE_H

			#include <android-base/thread_annotations.h>
	#include <cutils/misc.h>		#include <cutils/misc.h>
	#include <cutils/config_utils.h>		#include <cutils/config_utils.h>
	#include <cutils/compiler.h>		#include <cutils/compiler.h>
	@@ -330,13 +331,13 @@ private:
	AudioPolicyService() ANDROID_API;		AudioPolicyService() ANDROID_API;
	virtual ~AudioPolicyService();		virtual ~AudioPolicyService();

	status_t dumpInternals(int fd);		status_t dumpInternals(int fd) REQUIRES(mLock);

	// Handles binder shell commands		// Handles binder shell commands
	virtual status_t shellCommand(int in, int out, int err, Vector<String16>& args);		virtual status_t shellCommand(int in, int out, int err, Vector<String16>& args);

	// Sets whether the given UID records only silence		// 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		// Overrides the UID state as if it is idle
	status_t handleSetUidState(Vector<String16>& args, int err);		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);		status_t validateUsage(audio_usage_t usage, pid_t pid, uid_t uid);

	void updateUidStates();		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);		static bool isVirtualSource(audio_source_t source);

	@@ -420,13 +421,13 @@ private:
	wp<AudioPolicyService> mService;		wp<AudioPolicyService> mService;
	Mutex mLock;		Mutex mLock;
	ActivityManager mAm;		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>> mOverrideUids;
	std::unordered_map<uid_t, std::pair<bool, int>> mCachedUids;		std::unordered_map<uid_t, std::pair<bool, int>> mCachedUids;
	uid_t mAssistantUid;		uid_t mAssistantUid = -1;
	std::vector<uid_t> mA11yUids;		std::vector<uid_t> mA11yUids;
	uid_t mCurrentImeUid;		uid_t mCurrentImeUid = -1;
	bool mRttEnabled;		bool mRttEnabled = false;
	};		};

	// If sensor privacy is enabled then all apps, including those that are active, should be		// If sensor privacy is enabled then all apps, including those that are active, should be
	@@ -447,7 +448,7 @@ private:

	private:		private:
	wp<AudioPolicyService> mService;		wp<AudioPolicyService> mService;
	std::atomic_bool mSensorPrivacyEnabled;		std::atomic_bool mSensorPrivacyEnabled = false;
	};		};

	// Thread used to send audio config commands to audio flinger		// 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.		// and possibly back in to audio policy service and acquire mEffectsLock.
	sp<AudioCommandThread> mAudioCommandThread; // audio commands thread		sp<AudioCommandThread> mAudioCommandThread; // audio commands thread
	sp<AudioCommandThread> mOutputCommandThread; // process stop and release output		sp<AudioCommandThread> mOutputCommandThread; // process stop and release output
	struct audio_policy_device *mpAudioPolicyDev;
	struct audio_policy *mpAudioPolicy;
	AudioPolicyInterface *mAudioPolicyManager;		AudioPolicyInterface *mAudioPolicyManager;
	AudioPolicyClient *mAudioPolicyClient;		AudioPolicyClient *mAudioPolicyClient;
	std::vector<audio_usage_t> mSupportedSystemUsages;		std::vector<audio_usage_t> mSupportedSystemUsages;

	DefaultKeyedVector< int64_t, sp<NotificationClient> > mNotificationClients;		Mutex mNotificationClientsLock;
	Mutex mNotificationClientsLock; // protects mNotificationClients		DefaultKeyedVector<int64_t, sp<NotificationClient>> mNotificationClients
			GUARDED_BY(mNotificationClientsLock);
	// Manage all effects configured in audio_effects.conf		// Manage all effects configured in audio_effects.conf
	// never hold AudioPolicyService::mLock when calling AudioPolicyEffects methods as		// never hold AudioPolicyService::mLock when calling AudioPolicyEffects methods as
	// those can call back into AudioPolicyService methods and try to acquire the mutex		// those can call back into AudioPolicyService methods and try to acquire the mutex
	sp<AudioPolicyEffects> mAudioPolicyEffects;		sp<AudioPolicyEffects> mAudioPolicyEffects GUARDED_BY(mLock);
	audio_mode_t mPhoneState;		audio_mode_t mPhoneState GUARDED_BY(mLock);
	uid_t mPhoneStateOwnerUid;		uid_t mPhoneStateOwnerUid GUARDED_BY(mLock);

	sp<UidPolicy> mUidPolicy;		sp<UidPolicy> mUidPolicy GUARDED_BY(mLock);
	sp<SensorPrivacyPolicy> mSensorPrivacyPolicy;		sp<SensorPrivacyPolicy> mSensorPrivacyPolicy GUARDED_BY(mLock);

	DefaultKeyedVector< audio_port_handle_t, sp<AudioRecordClient> > mAudioRecordClients;		DefaultKeyedVector<audio_port_handle_t, sp<AudioRecordClient>> mAudioRecordClients
	DefaultKeyedVector< audio_port_handle_t, sp<AudioPlaybackClient> > mAudioPlaybackClients;		GUARDED_BY(mLock);
			DefaultKeyedVector<audio_port_handle_t, sp<AudioPlaybackClient>> mAudioPlaybackClients
			GUARDED_BY(mLock);

	MediaPackageManager mPackageManager; // To check allowPlaybackCapture		MediaPackageManager mPackageManager; // To check allowPlaybackCapture