Merge "camera: Fix client eviction/disconnect race." (4264bbd7) · Commits · e / os / android_frameworks_av

services/camera/libcameraservice/CameraService.cpp

+23 −4

Original line number	Diff line number	Diff line
		@@ -866,7 +866,7 @@ status_t CameraService::handleEvictionsLocked(const String8& cameraId, int clien
		std::shared_ptr<resource_policy::ClientDescriptor<String8, sp<BasicClient>>>* partial) {

		status_t ret = NO_ERROR;
		std::vector<sp<BasicClient>> evictedClients;
		std::vector<DescriptorPtr> evictedClients;
		DescriptorPtr clientDescriptor;
		{
		if (effectiveApiLevel == API_1) {
		@@ -974,7 +974,7 @@ status_t CameraService::handleEvictionsLocked(const String8& cameraId, int clien

		ALOGE("CameraService::connect evicting conflicting client for camera ID %s",
		i->getKey().string());
		evictedClients.push_back(clientSp);
		evictedClients.push_back(i);

		// Log the clients evicted
		logEvent(String8::format("EVICT device %s client held by package %s (PID"
		@@ -1001,12 +1001,31 @@ status_t CameraService::handleEvictionsLocked(const String8& cameraId, int clien
		// Destroy evicted clients
		for (auto& i : evictedClients) {
		// Disconnect is blocking, and should only have returned when HAL has cleaned up
		i->disconnect(); // Clients will remove themselves from the active client list here
		i->getValue()->disconnect(); // Clients will remove themselves from the active client list
		}
		evictedClients.clear();

		IPCThreadState::self()->restoreCallingIdentity(token);

		for (const auto& i : evictedClients) {
		ALOGV("%s: Waiting for disconnect to complete for client for device %s (PID %" PRId32 ")",
		__FUNCTION__, i->getKey().string(), i->getOwnerId());
		ret = mActiveClientManager.waitUntilRemoved(i, DEFAULT_DISCONNECT_TIMEOUT_NS);
		if (ret == TIMED_OUT) {
		ALOGE("%s: Timed out waiting for client for device %s to disconnect, "
		"current clients:\n%s", __FUNCTION__, i->getKey().string(),
		mActiveClientManager.toString().string());
		return -EBUSY;
		}
		if (ret != NO_ERROR) {
		ALOGE("%s: Received error waiting for client for device %s to disconnect: %s (%d), "
		"current clients:\n%s", __FUNCTION__, i->getKey().string(), strerror(-ret),
		ret, mActiveClientManager.toString().string());
		return ret;
		}
		}

		evictedClients.clear();

		// Once clients have been disconnected, relock
		mServiceLock.lock();

services/camera/libcameraservice/CameraService.h

+3 −0

Original line number	Diff line number	Diff line
		@@ -90,6 +90,9 @@ public:
		// 3 second busy timeout when other clients are connecting
		static const nsecs_t DEFAULT_CONNECT_TIMEOUT_NS = 3000000000;

		// 1 second busy timeout when other clients are disconnecting
		static const nsecs_t DEFAULT_DISCONNECT_TIMEOUT_NS = 1000000000;

		// Default number of messages to store in eviction log
		static const size_t DEFAULT_EVENT_LOG_LENGTH = 100;

services/camera/libcameraservice/utils/ClientManager.h

+49 −0

Original line number	Diff line number	Diff line
		@@ -17,7 +17,9 @@
		#ifndef ANDROID_SERVICE_UTILS_EVICTION_POLICY_MANAGER_H
		#define ANDROID_SERVICE_UTILS_EVICTION_POLICY_MANAGER_H

		#include <utils/Condition.h>
		#include <utils/Mutex.h>
		#include <utils/Timers.h>

		#include <algorithm>
		#include <utility>
		@@ -263,6 +265,16 @@ public:
		*/
		std::shared_ptr<ClientDescriptor<KEY, VALUE>> get(const KEY& key) const;

		/**
		* Block until the given client is no longer in the active clients list, or the timeout
		* occurred.
		*
		* Returns NO_ERROR if this succeeded, -ETIMEDOUT on a timeout, or a negative error code on
		* failure.
		*/
		status_t waitUntilRemoved(const std::shared_ptr<ClientDescriptor<KEY, VALUE>> client,
		nsecs_t timeout) const;

		protected:
		~ClientManager();

		@@ -284,6 +296,7 @@ private:
		int64_t getCurrentCostLocked() const;

		mutable Mutex mLock;
		mutable Condition mRemovedCondition;
		int32_t mMaxCost;
		// LRU ordered, most recent at end
		std::vector<std::shared_ptr<ClientDescriptor<KEY, VALUE>>> mClients;
		@@ -430,6 +443,7 @@ std::vector<std::shared_ptr<ClientDescriptor<KEY, VALUE>>> ClientManager<KEY, VA
		}), mClients.end());

		mClients.push_back(client);
		mRemovedCondition.broadcast();

		return evicted;
		}
		@@ -487,6 +501,7 @@ template<class KEY, class VALUE>
		void ClientManager<KEY, VALUE>::removeAll() {
		Mutex::Autolock lock(mLock);
		mClients.clear();
		mRemovedCondition.broadcast();
		}

		template<class KEY, class VALUE>
		@@ -505,6 +520,39 @@ std::shared_ptr<ClientDescriptor<KEY, VALUE>> ClientManager<KEY, VALUE>::remove(
		return false;
		}), mClients.end());

		mRemovedCondition.broadcast();
		return ret;
		}

		template<class KEY, class VALUE>
		status_t ClientManager<KEY, VALUE>::waitUntilRemoved(
		const std::shared_ptr<ClientDescriptor<KEY, VALUE>> client,
		nsecs_t timeout) const {
		status_t ret = NO_ERROR;
		Mutex::Autolock lock(mLock);

		bool isRemoved = false;

		// Figure out what time in the future we should hit the timeout
		nsecs_t failTime = systemTime(SYSTEM_TIME_MONOTONIC) + timeout;

		while (!isRemoved) {
		isRemoved = true;
		for (const auto& i : mClients) {
		if (i == client) {
		isRemoved = false;
		}
		}

		if (!isRemoved) {
		ret = mRemovedCondition.waitRelative(mLock, timeout);
		if (ret != NO_ERROR) {
		break;
		}
		timeout = failTime - systemTime(SYSTEM_TIME_MONOTONIC);
		}
		}

		return ret;
		}

		@@ -520,6 +568,7 @@ void ClientManager<KEY, VALUE>::remove(
		}
		return false;
		}), mClients.end());
		mRemovedCondition.broadcast();
		}

		template<class KEY, class VALUE>