Merge "cameraserver: Avoiding deadlocks while calling getSystemCameraKind()" (347b9799) · Commits · e / os / android_frameworks_av

services/camera/libcameraservice/CameraService.cpp

+77 −19

Original line number	Diff line number	Diff line
		@@ -268,8 +268,12 @@ void CameraService::filterAPI1SystemCameraLocked(
		const std::vector<std::string> &normalDeviceIds) {
		mNormalDeviceIdsWithoutSystemCamera.clear();
		for (auto &deviceId : normalDeviceIds) {
		if (getSystemCameraKind(String8(deviceId.c_str())) ==
		SystemCameraKind::SYSTEM_ONLY_CAMERA) {
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKind(String8(deviceId.c_str()), &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, deviceId.c_str());
		continue;
		}
		if (deviceKind == SystemCameraKind::SYSTEM_ONLY_CAMERA) {
		// All system camera ids will necessarily come after public camera
		// device ids as per the HAL interface contract.
		break;
		@@ -280,6 +284,16 @@ void CameraService::filterAPI1SystemCameraLocked(
		mNormalDeviceIdsWithoutSystemCamera.size());
		}

		status_t CameraService::getSystemCameraKind(const String8& cameraId, SystemCameraKind *kind) const {
		auto state = getCameraState(cameraId);
		if (state != nullptr) {
		*kind = state->getSystemCameraKind();
		return OK;
		}
		// Hidden physical camera ids won't have CameraState
		return mCameraProviderManager->getSystemCameraKind(cameraId.c_str(), kind);
		}

		void CameraService::updateCameraNumAndIds() {
		Mutex::Autolock l(mServiceLock);
		std::pair<int, int> systemAndNonSystemCameras = mCameraProviderManager->getCameraCount();
		@@ -296,10 +310,16 @@ void CameraService::addStates(const String8 id) {
		std::string cameraId(id.c_str());
		hardware::camera::common::V1_0::CameraResourceCost cost;
		status_t res = mCameraProviderManager->getResourceCost(cameraId, &cost);
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (res != OK) {
		ALOGE("Failed to query device resource cost: %s (%d)", strerror(-res), res);
		return;
		}
		res = mCameraProviderManager->getSystemCameraKind(cameraId, &deviceKind);
		if (res != OK) {
		ALOGE("Failed to query device kind: %s (%d)", strerror(-res), res);
		return;
		}
		std::set<String8> conflicting;
		for (size_t i = 0; i < cost.conflictingDevices.size(); i++) {
		conflicting.emplace(String8(cost.conflictingDevices[i].c_str()));
		@@ -308,7 +328,7 @@ void CameraService::addStates(const String8 id) {
		{
		Mutex::Autolock lock(mCameraStatesLock);
		mCameraStates.emplace(id, std::make_shared<CameraState>(id, cost.resourceCost,
		conflicting));
		conflicting, deviceKind));
		}

		if (mFlashlight->hasFlashUnit(id)) {
		@@ -594,7 +614,12 @@ Status CameraService::getCameraCharacteristics(const String16& cameraId,
		"characteristics for device %s: %s (%d)", String8(cameraId).string(),
		strerror(-res), res);
		}

		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKind(String8(cameraId), &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, String8(cameraId).string());
		return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION, "Unable to retrieve camera kind "
		"for device %s", String8(cameraId).string());
		}
		int callingPid = CameraThreadState::getCallingPid();
		int callingUid = CameraThreadState::getCallingUid();
		std::vector<int32_t> tagsRemoved;
		@@ -602,7 +627,7 @@ Status CameraService::getCameraCharacteristics(const String16& cameraId,
		// android.permission.CAMERA is required. If android.permission.SYSTEM_CAMERA was needed,
		// it would've already been checked in shouldRejectSystemCameraConnection.
		if ((callingPid != getpid()) &&
		(getSystemCameraKind(String8(cameraId)) != SystemCameraKind::SYSTEM_ONLY_CAMERA) &&
		(deviceKind != SystemCameraKind::SYSTEM_ONLY_CAMERA) &&
		!checkPermission(sCameraPermission, callingPid, callingUid)) {
		res = cameraInfo->removePermissionEntries(
		mCameraProviderManager->getProviderTagIdLocked(String8(cameraId).string()),
		@@ -1049,11 +1074,19 @@ Status CameraService::validateClientPermissionsLocked(const String8& cameraId,
		return STATUS_ERROR_FMT(ERROR_DISCONNECTED, "No camera device with ID \"%s\" is"
		"available", cameraId.string());
		}
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKind(cameraId, &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, cameraId.string());
		return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT, "No camera device with ID \"%s\""
		"found while trying to query device kind", cameraId.string());

		}

		// If it's not calling from cameraserver, check the permission if the
		// device isn't a system only camera (shouldRejectSystemCameraConnection already checks for
		// android.permission.SYSTEM_CAMERA for system only camera devices).
		if (callingPid != getpid() &&
		(getSystemCameraKind(cameraId) != SystemCameraKind::SYSTEM_ONLY_CAMERA) &&
		(deviceKind != SystemCameraKind::SYSTEM_ONLY_CAMERA) &&
		!checkPermission(sCameraPermission, clientPid, clientUid)) {
		ALOGE("Permission Denial: can't use the camera pid=%d, uid=%d", clientPid, clientUid);
		return STATUS_ERROR_FMT(ERROR_PERMISSION_DENIED,
		@@ -1407,9 +1440,8 @@ Status CameraService::connectLegacy(
		return ret;
		}

		bool CameraService::shouldSkipStatusUpdates(const String8& cameraId, bool isVendorListener,
		int clientPid, int clientUid) const {
		SystemCameraKind systemCameraKind = getSystemCameraKind(cameraId);
		bool CameraService::shouldSkipStatusUpdates(SystemCameraKind systemCameraKind,
		bool isVendorListener, int clientPid, int clientUid) {
		// If the client is not a vendor client, don't add listener if
		// a) the camera is a publicly hidden secure camera OR
		// b) the camera is a system only camera and the client doesn't
		@@ -1435,7 +1467,11 @@ bool CameraService::shouldRejectSystemCameraConnection(const String8& cameraId)

		int cPid = CameraThreadState::getCallingPid();
		int cUid = CameraThreadState::getCallingUid();
		SystemCameraKind systemCameraKind = getSystemCameraKind(cameraId);
		SystemCameraKind systemCameraKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKind(cameraId, &systemCameraKind) != OK) {
		ALOGE("%s: Invalid camera id %s, ", __FUNCTION__, cameraId.c_str());
		return true;
		}

		// (1) Cameraserver trying to connect, accept.
		if (CameraThreadState::getCallingPid() == getpid()) {
		@@ -1926,14 +1962,24 @@ Status CameraService::addListenerHelper(const sp<ICameraServiceListener>& listen
		{
		Mutex::Autolock lock(mCameraStatesLock);
		for (auto& i : mCameraStates) {
		if (shouldSkipStatusUpdates(i.first, isVendorListener, clientPid, clientUid)) {
		ALOGV("Cannot add public listener for hidden system-only %s for pid %d",
		i.first.c_str(), CameraThreadState::getCallingPid());
		continue;
		}
		cameraStatuses->emplace_back(i.first, mapToInterface(i.second->getStatus()));
		}
		}
		// Remove the camera statuses that should be hidden from the client, we do
		// this after collecting the states in order to avoid holding
		// mCameraStatesLock and mInterfaceLock (held in getSystemCameraKind()) at
		// the same time.
		cameraStatuses->erase(std::remove_if(cameraStatuses->begin(), cameraStatuses->end(),
		[this, &isVendorListener, &clientPid, &clientUid](const hardware::CameraStatus& s) {
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKind(s.cameraId, &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping status update",
		__FUNCTION__, s.cameraId.c_str());
		return true;
		}
		return shouldSkipStatusUpdates(deviceKind, isVendorListener, clientPid,
		clientUid);}), cameraStatuses->end());


		/*
		* Immediately signal current torch status to this listener only
		@@ -3023,8 +3069,9 @@ void CameraService::SensorPrivacyPolicy::binderDied(const wp<IBinder>& /who/)
		// ----------------------------------------------------------------------------

		CameraService::CameraState::CameraState(const String8& id, int cost,
		const std::set<String8>& conflicting) : mId(id),
		mStatus(StatusInternal::NOT_PRESENT), mCost(cost), mConflicting(conflicting) {}
		const std::set<String8>& conflicting, SystemCameraKind systemCameraKind) : mId(id),
		mStatus(StatusInternal::NOT_PRESENT), mCost(cost), mConflicting(conflicting),
		mSystemCameraKind(systemCameraKind) {}

		CameraService::CameraState::~CameraState() {}

		@@ -3053,6 +3100,10 @@ String8 CameraService::CameraState::getId() const {
		return mId;
		}

		SystemCameraKind CameraService::CameraState::getSystemCameraKind() const {
		return mSystemCameraKind;
		}

		// ----------------------------------------------------------------------------
		// ClientEventListener
		// ----------------------------------------------------------------------------
		@@ -3391,9 +3442,16 @@ void CameraService::updateStatus(StatusInternal status, const String8& cameraId,
		return;
		}

		// Avoid calling getSystemCameraKind() with mStatusListenerLock held (b/141756275)
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKind(cameraId, &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, cameraId.string());
		return;
		}

		// Update the status for this camera state, then send the onStatusChangedCallbacks to each
		// of the listeners with both the mStatusStatus and mStatusListenerLock held
		state->updateStatus(status, cameraId, rejectSourceStates, [this]
		state->updateStatus(status, cameraId, rejectSourceStates, [this, &deviceKind]
		(const String8& cameraId, StatusInternal status) {

		if (status != StatusInternal::ENUMERATING) {
		@@ -3415,7 +3473,7 @@ void CameraService::updateStatus(StatusInternal status, const String8& cameraId,
		Mutex::Autolock lock(mStatusListenerLock);

		for (auto& listener : mListenerList) {
		if (shouldSkipStatusUpdates(cameraId, listener->isVendorListener(),
		if (shouldSkipStatusUpdates(deviceKind, listener->isVendorListener(),
		listener->getListenerPid(), listener->getListenerUid())) {
		ALOGV("Skipping camera discovery callback for system-only camera %s",
		cameraId.c_str());

services/camera/libcameraservice/CameraService.h

+16 −7

Original line number	Diff line number	Diff line
		@@ -490,7 +490,8 @@ private:
		* Make a new CameraState and set the ID, cost, and conflicting devices using the values
		* returned in the HAL's camera_info struct for each device.
		*/
		CameraState(const String8& id, int cost, const std::set<String8>& conflicting);
		CameraState(const String8& id, int cost, const std::set<String8>& conflicting,
		SystemCameraKind deviceKind);
		virtual ~CameraState();

		/**
		@@ -542,6 +543,11 @@ private:
		*/
		String8 getId() const;

		/**
		* Return the kind (SystemCameraKind) of this camera device.
		*/
		SystemCameraKind getSystemCameraKind() const;

		private:
		const String8 mId;
		StatusInternal mStatus; // protected by mStatusLock
		@@ -549,6 +555,7 @@ private:
		std::set<String8> mConflicting;
		mutable Mutex mStatusLock;
		CameraParameters mShimParams;
		const SystemCameraKind mSystemCameraKind;
		}; // class CameraState

		// Observer for UID lifecycle enforcing that UIDs in idle
		@@ -660,12 +667,14 @@ private:
		// Should a device status update be skipped for a particular camera device ? (this can happen
		// under various conditions. For example if a camera device is advertised as
		// system only or hidden secure camera, amongst possible others.
		bool shouldSkipStatusUpdates(const String8& cameraId, bool isVendorListener, int clientPid,
		int clientUid) const;

		inline SystemCameraKind getSystemCameraKind(const String8& cameraId) const {
		return mCameraProviderManager->getSystemCameraKind(cameraId.c_str());
		}
		static bool shouldSkipStatusUpdates(SystemCameraKind systemCameraKind, bool isVendorListener,
		int clientPid, int clientUid);

		// Gets the kind of camera device (i.e public, hidden secure or system only)
		// getSystemCameraKind() needs mInterfaceMutex which might lead to deadlocks
		// if held along with mStatusListenerLock (depending on lock ordering, b/141756275), it is
		// recommended that we don't call this function with mStatusListenerLock held.
		status_t getSystemCameraKind(const String8& cameraId, SystemCameraKind *kind) const;

		// Update the set of API1Compatible camera devices without including system
		// cameras and secure cameras. This is used for hiding system only cameras

services/camera/libcameraservice/common/CameraProviderManager.cpp

+50 −17

Original line number	Diff line number	Diff line
		@@ -110,7 +110,12 @@ std::pair<int, int> CameraProviderManager::getCameraCount() const {
		int publicCameraCount = 0;
		for (auto& provider : mProviders) {
		for (auto &id : provider->mUniqueCameraIds) {
		switch(getSystemCameraKindLocked(id)) {
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKindLocked(id, &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, id.c_str());
		continue;
		}
		switch(deviceKind) {
		case SystemCameraKind::PUBLIC:
		publicCameraCount++;
		break;
		@@ -140,7 +145,12 @@ void CameraProviderManager::collectDeviceIdsLocked(const std::vector<std::string
		std::vector<std::string>& publicDeviceIds,
		std::vector<std::string>& systemDeviceIds) const {
		for (auto &deviceId : deviceIds) {
		if (getSystemCameraKindLocked(deviceId) == SystemCameraKind::SYSTEM_ONLY_CAMERA) {
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKindLocked(deviceId, &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, deviceId.c_str());
		continue;
		}
		if (deviceKind == SystemCameraKind::SYSTEM_ONLY_CAMERA) {
		systemDeviceIds.push_back(deviceId);
		} else {
		publicDeviceIds.push_back(deviceId);
		@@ -158,9 +168,13 @@ std::vector<std::string> CameraProviderManager::getAPI1CompatibleCameraDeviceIds
		// Secure cameras should not be exposed through camera 1 api
		providerDeviceIds.erase(std::remove_if(providerDeviceIds.begin(), providerDeviceIds.end(),
		[this](const std::string& s) {
		bool rem = this->getSystemCameraKindLocked(s) ==
		SystemCameraKind::HIDDEN_SECURE_CAMERA;
		return rem;}), providerDeviceIds.end());
		SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
		if (getSystemCameraKindLocked(s, &deviceKind) != OK) {
		ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, s.c_str());
		return true;
		}
		return deviceKind == SystemCameraKind::HIDDEN_SECURE_CAMERA;}),
		providerDeviceIds.end());
		// API1 app doesn't handle logical and physical camera devices well. So
		// for each camera facing, only take the first id advertised by HAL in
		// all [logical, physical1, physical2, ...] id combos, and filter out the rest.
		@@ -1089,26 +1103,45 @@ bool CameraProviderManager::isLogicalCamera(const std::string& id,
		return deviceInfo->mIsLogicalCamera;
		}

		SystemCameraKind CameraProviderManager::getSystemCameraKind(const std::string& id) const {
		status_t CameraProviderManager::getSystemCameraKind(const std::string& id,
		SystemCameraKind *kind) const {
		std::lock_guard<std::mutex> lock(mInterfaceMutex);
		return getSystemCameraKindLocked(id);
		return getSystemCameraKindLocked(id, kind);
		}

		SystemCameraKind CameraProviderManager::getSystemCameraKindLocked(const std::string& id) const {
		status_t CameraProviderManager::getSystemCameraKindLocked(const std::string& id,
		SystemCameraKind *kind) const {
		auto deviceInfo = findDeviceInfoLocked(id);
		if (deviceInfo == nullptr) {
		return SystemCameraKind::PUBLIC;
		if (deviceInfo != nullptr) {
		*kind = deviceInfo->mSystemCameraKind;
		return OK;
		}
		return deviceInfo->mSystemCameraKind;
		// If this is a hidden physical camera, we should return what kind of
		// camera the enclosing logical camera is.
		auto isHiddenAndParent = isHiddenPhysicalCameraInternal(id);
		if (isHiddenAndParent.first) {
		LOG_ALWAYS_FATAL_IF(id == isHiddenAndParent.second->mId,
		"%s: hidden physical camera id %s and enclosing logical camera id %s are the same",
		__FUNCTION__, id.c_str(), isHiddenAndParent.second->mId.c_str());
		return getSystemCameraKindLocked(isHiddenAndParent.second->mId, kind);
		}
		// Neither a hidden physical camera nor a logical camera
		return NAME_NOT_FOUND;
		}

		bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) {
		bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) const {
		return isHiddenPhysicalCameraInternal(cameraId).first;
		}

		std::pair<bool, CameraProviderManager::ProviderInfo::DeviceInfo *>
		CameraProviderManager::isHiddenPhysicalCameraInternal(const std::string& cameraId) const {
		auto falseRet = std::make_pair(false, nullptr);
		for (auto& provider : mProviders) {
		for (auto& deviceInfo : provider->mDevices) {
		if (deviceInfo->mId == cameraId) {
		// cameraId is found in public camera IDs advertised by the
		// provider.
		return false;
		return falseRet;
		}
		}
		}
		@@ -1120,7 +1153,7 @@ bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId)
		if (res != OK) {
		ALOGE("%s: Failed to getCameraCharacteristics for id %s", __FUNCTION__,
		deviceInfo->mId.c_str());
		return false;
		return falseRet;
		}

		std::vector<std::string> physicalIds;
		@@ -1132,16 +1165,16 @@ bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId)
		if (deviceVersion < CAMERA_DEVICE_API_VERSION_3_5) {
		ALOGE("%s: Wrong deviceVersion %x for hiddenPhysicalCameraId %s",
		__FUNCTION__, deviceVersion, cameraId.c_str());
		return false;
		return falseRet;
		} else {
		return true;
		return std::make_pair(true, deviceInfo.get());
		}
		}
		}
		}
		}

		return false;
		return falseRet;
		}

		status_t CameraProviderManager::addProviderLocked(const std::string& newProvider) {

services/camera/libcameraservice/common/CameraProviderManager.h

+4 −3

Original line number	Diff line number	Diff line
		@@ -292,8 +292,8 @@ public:
		*/
		bool isLogicalCamera(const std::string& id, std::vector<std::string>* physicalCameraIds);

		SystemCameraKind getSystemCameraKind(const std::string& id) const;
		bool isHiddenPhysicalCamera(const std::string& cameraId);
		status_t getSystemCameraKind(const std::string& id, SystemCameraKind *kind) const;
		bool isHiddenPhysicalCamera(const std::string& cameraId) const;

		static const float kDepthARTolerance;
		private:
		@@ -616,7 +616,8 @@ private:
		CameraMetadata* characteristics) const;
		void filterLogicalCameraIdsLocked(std::vector<std::string>& deviceIds) const;

		SystemCameraKind getSystemCameraKindLocked(const std::string& id) const;
		status_t getSystemCameraKindLocked(const std::string& id, SystemCameraKind *kind) const;
		std::pair<bool, ProviderInfo::DeviceInfo *> isHiddenPhysicalCameraInternal(const std::string& cameraId) const;

		void collectDeviceIdsLocked(const std::vector<std::string> deviceIds,
		std::vector<std::string>& normalDeviceIds,