Merge "Camera: Fix race between callback and unregisteration of callbacks" into rvc-dev (f8673bf0) · Commits · e / os / android_frameworks_av

camera/ndk/impl/ACameraManager.cpp

+49 −1

Original line number	Original line	Diff line number	Diff line
	@@ -35,6 +35,7 @@ const char* CameraManagerGlobal::kCameraIdKey = "CameraId";
	const char* CameraManagerGlobal::kPhysicalCameraIdKey = "PhysicalCameraId";		const char* CameraManagerGlobal::kPhysicalCameraIdKey = "PhysicalCameraId";
	const char* CameraManagerGlobal::kCallbackFpKey = "CallbackFp";		const char* CameraManagerGlobal::kCallbackFpKey = "CallbackFp";
	const char* CameraManagerGlobal::kContextKey = "CallbackContext";		const char* CameraManagerGlobal::kContextKey = "CallbackContext";
			const nsecs_t CameraManagerGlobal::kCallbackDrainTimeout = 5000000; // 5 ms
	Mutex CameraManagerGlobal::sLock;		Mutex CameraManagerGlobal::sLock;
	CameraManagerGlobal* CameraManagerGlobal::sInstance = nullptr;		CameraManagerGlobal* CameraManagerGlobal::sInstance = nullptr;

	@@ -117,7 +118,7 @@ sp<hardware::ICameraService> CameraManagerGlobal::getCameraServiceLocked() {
	return nullptr;		return nullptr;
	}		}
	if (mHandler == nullptr) {		if (mHandler == nullptr) {
	mHandler = new CallbackHandler();		mHandler = new CallbackHandler(this);
	}		}
	mCbLooper->registerHandler(mHandler);		mCbLooper->registerHandler(mHandler);
	}		}
	@@ -211,6 +212,9 @@ void CameraManagerGlobal::registerExtendedAvailabilityCallback(
	void CameraManagerGlobal::unregisterExtendedAvailabilityCallback(		void CameraManagerGlobal::unregisterExtendedAvailabilityCallback(
	const ACameraManager_ExtendedAvailabilityCallbacks *callback) {		const ACameraManager_ExtendedAvailabilityCallbacks *callback) {
	Mutex::Autolock _l(mLock);		Mutex::Autolock _l(mLock);

			drainPendingCallbacksLocked();

	Callback cb(callback);		Callback cb(callback);
	mCallbacks.erase(cb);		mCallbacks.erase(cb);
	}		}
	@@ -223,10 +227,32 @@ void CameraManagerGlobal::registerAvailabilityCallback(
	void CameraManagerGlobal::unregisterAvailabilityCallback(		void CameraManagerGlobal::unregisterAvailabilityCallback(
	const ACameraManager_AvailabilityCallbacks *callback) {		const ACameraManager_AvailabilityCallbacks *callback) {
	Mutex::Autolock _l(mLock);		Mutex::Autolock _l(mLock);

			drainPendingCallbacksLocked();

	Callback cb(callback);		Callback cb(callback);
	mCallbacks.erase(cb);		mCallbacks.erase(cb);
	}		}

			void CameraManagerGlobal::onCallbackCalled() {
			Mutex::Autolock _l(mLock);
			if (mPendingCallbackCnt > 0) {
			mPendingCallbackCnt--;
			}
			mCallbacksCond.signal();
			}

			void CameraManagerGlobal::drainPendingCallbacksLocked() {
			while (mPendingCallbackCnt > 0) {
			auto res = mCallbacksCond.waitRelative(mLock, kCallbackDrainTimeout);
			if (res != NO_ERROR) {
			ALOGE("%s: Error waiting to drain callbacks: %s(%d)",
			__FUNCTION__, strerror(-res), res);
			break;
			}
			}
			}

	template<class T>		template<class T>
	void CameraManagerGlobal::registerAvailCallback(const T *callback) {		void CameraManagerGlobal::registerAvailCallback(const T *callback) {
	Mutex::Autolock _l(mLock);		Mutex::Autolock _l(mLock);
	@@ -250,6 +276,7 @@ void CameraManagerGlobal::registerAvailCallback(const T *callback) {
	msg->setPointer(kCallbackFpKey, (void *) cbFunc);		msg->setPointer(kCallbackFpKey, (void *) cbFunc);
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId));		msg->setString(kCameraIdKey, AString(cameraId));
			mPendingCallbackCnt++;
	msg->post();		msg->post();

	// Physical camera unavailable callback		// Physical camera unavailable callback
	@@ -263,6 +290,7 @@ void CameraManagerGlobal::registerAvailCallback(const T *callback) {
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId));		msg->setString(kCameraIdKey, AString(cameraId));
	msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId));		msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId));
			mPendingCallbackCnt++;
	msg->post();		msg->post();
	}		}
	}		}
	@@ -324,6 +352,16 @@ bool CameraManagerGlobal::isStatusAvailable(int32_t status) {

	void CameraManagerGlobal::CallbackHandler::onMessageReceived(		void CameraManagerGlobal::CallbackHandler::onMessageReceived(
	const sp<AMessage> &msg) {		const sp<AMessage> &msg) {
			onMessageReceivedInternal(msg);
			if (msg->what() == kWhatSendSingleCallback \|\|
			msg->what() == kWhatSendSingleAccessCallback \|\|
			msg->what() == kWhatSendSinglePhysicalCameraCallback) {
			notifyParent();
			}
			}

			void CameraManagerGlobal::CallbackHandler::onMessageReceivedInternal(
			const sp<AMessage> &msg) {
	switch (msg->what()) {		switch (msg->what()) {
	case kWhatSendSingleCallback:		case kWhatSendSingleCallback:
	{		{
	@@ -405,6 +443,13 @@ void CameraManagerGlobal::CallbackHandler::onMessageReceived(
	}		}
	}		}

			void CameraManagerGlobal::CallbackHandler::notifyParent() {
			sp<CameraManagerGlobal> parent = mParent.promote();
			if (parent != nullptr) {
			parent->onCallbackCalled();
			}
			}

	binder::Status CameraManagerGlobal::CameraServiceListener::onCameraAccessPrioritiesChanged() {		binder::Status CameraManagerGlobal::CameraServiceListener::onCameraAccessPrioritiesChanged() {
	sp<CameraManagerGlobal> cm = mCameraManager.promote();		sp<CameraManagerGlobal> cm = mCameraManager.promote();
	if (cm != nullptr) {		if (cm != nullptr) {
	@@ -445,6 +490,7 @@ void CameraManagerGlobal::onCameraAccessPrioritiesChanged() {
	if (cbFp != nullptr) {		if (cbFp != nullptr) {
	msg->setPointer(kCallbackFpKey, (void *) cbFp);		msg->setPointer(kCallbackFpKey, (void *) cbFp);
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
			mPendingCallbackCnt++;
	msg->post();		msg->post();
	}		}
	}		}
	@@ -491,6 +537,7 @@ void CameraManagerGlobal::onStatusChangedLocked(
	msg->setPointer(kCallbackFpKey, (void *) cbFp);		msg->setPointer(kCallbackFpKey, (void *) cbFp);
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId));		msg->setString(kCameraIdKey, AString(cameraId));
			mPendingCallbackCnt++;
	msg->post();		msg->post();
	}		}
	}		}
	@@ -545,6 +592,7 @@ void CameraManagerGlobal::onStatusChangedLocked(
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId));		msg->setString(kCameraIdKey, AString(cameraId));
	msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId));		msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId));
			mPendingCallbackCnt++;
	msg->post();		msg->post();
	}		}
	}		}

camera/ndk/impl/ACameraManager.h

+13 −1

Original line number	Original line	Diff line number	Diff line
	@@ -163,6 +163,12 @@ class CameraManagerGlobal final : public RefBase {
	ACameraManager_PhysicalCameraAvailabilityCallback mPhysicalCamUnavailable;		ACameraManager_PhysicalCameraAvailabilityCallback mPhysicalCamUnavailable;
	void* mContext;		void* mContext;
	};		};

			android::Condition mCallbacksCond;
			size_t mPendingCallbackCnt = 0;
			void onCallbackCalled();
			void drainPendingCallbacksLocked();

	std::set<Callback> mCallbacks;		std::set<Callback> mCallbacks;

	// definition of handler and message		// definition of handler and message
	@@ -175,10 +181,16 @@ class CameraManagerGlobal final : public RefBase {
	static const char* kPhysicalCameraIdKey;		static const char* kPhysicalCameraIdKey;
	static const char* kCallbackFpKey;		static const char* kCallbackFpKey;
	static const char* kContextKey;		static const char* kContextKey;
			static const nsecs_t kCallbackDrainTimeout;
	class CallbackHandler : public AHandler {		class CallbackHandler : public AHandler {
	public:		public:
	CallbackHandler() {}		CallbackHandler(wp<CameraManagerGlobal> parent) : mParent(parent) {}
	void onMessageReceived(const sp<AMessage> &msg) override;		void onMessageReceived(const sp<AMessage> &msg) override;

			private:
			wp<CameraManagerGlobal> mParent;
			void notifyParent();
			void onMessageReceivedInternal(const sp<AMessage> &msg);
	};		};
	sp<CallbackHandler> mHandler;		sp<CallbackHandler> mHandler;
	sp<ALooper> mCbLooper; // Looper thread where callbacks actually happen on		sp<ALooper> mCbLooper; // Looper thread where callbacks actually happen on

camera/ndk/include/camera/NdkCameraManager.h

+6 −0

Original line number	Original line	Diff line number	Diff line
	@@ -191,6 +191,9 @@ camera_status_t ACameraManager_registerAvailabilityCallback(
	*		*
	* <p>Removing a callback that isn't registered has no effect.</p>		* <p>Removing a callback that isn't registered has no effect.</p>
	*		*
			* <p>This function must not be called with a mutex lock also held by
			* the availability callbacks.</p>
			*
	* @param manager the {@link ACameraManager} of interest.		* @param manager the {@link ACameraManager} of interest.
	* @param callback the {@link ACameraManager_AvailabilityCallbacks} to be unregistered.		* @param callback the {@link ACameraManager_AvailabilityCallbacks} to be unregistered.
	*		*
	@@ -382,6 +385,9 @@ camera_status_t ACameraManager_registerExtendedAvailabilityCallback(
	*		*
	* <p>Removing a callback that isn't registered has no effect.</p>		* <p>Removing a callback that isn't registered has no effect.</p>
	*		*
			* <p>This function must not be called with a mutex lock also held by
			* the extended availability callbacks.</p>
			*
	* @param manager the {@link ACameraManager} of interest.		* @param manager the {@link ACameraManager} of interest.
	* @param callback the {@link ACameraManager_ExtendedAvailabilityCallbacks} to be unregistered.		* @param callback the {@link ACameraManager_ExtendedAvailabilityCallbacks} to be unregistered.
	*		*

camera/ndk/ndk_vendor/impl/ACameraManager.cpp

+43 −1

Original line number	Original line	Diff line number	Diff line
	@@ -45,6 +45,7 @@ const char* CameraManagerGlobal::kCameraIdKey = "CameraId";
	const char* CameraManagerGlobal::kPhysicalCameraIdKey = "PhysicalCameraId";		const char* CameraManagerGlobal::kPhysicalCameraIdKey = "PhysicalCameraId";
	const char* CameraManagerGlobal::kCallbackFpKey = "CallbackFp";		const char* CameraManagerGlobal::kCallbackFpKey = "CallbackFp";
	const char* CameraManagerGlobal::kContextKey = "CallbackContext";		const char* CameraManagerGlobal::kContextKey = "CallbackContext";
			const nsecs_t CameraManagerGlobal::kCallbackDrainTimeout = 5000000; // 5 ms
	Mutex CameraManagerGlobal::sLock;		Mutex CameraManagerGlobal::sLock;
	CameraManagerGlobal* CameraManagerGlobal::sInstance = nullptr;		CameraManagerGlobal* CameraManagerGlobal::sInstance = nullptr;

	@@ -249,7 +250,7 @@ sp<ICameraService> CameraManagerGlobal::getCameraService() {
	return nullptr;		return nullptr;
	}		}
	if (mHandler == nullptr) {		if (mHandler == nullptr) {
	mHandler = new CallbackHandler();		mHandler = new CallbackHandler(this);
	}		}
	mCbLooper->registerHandler(mHandler);		mCbLooper->registerHandler(mHandler);
	}		}
	@@ -317,6 +318,7 @@ void CameraManagerGlobal::registerAvailabilityCallback(
	void CameraManagerGlobal::unregisterAvailabilityCallback(		void CameraManagerGlobal::unregisterAvailabilityCallback(
	const ACameraManager_AvailabilityCallbacks *callback) {		const ACameraManager_AvailabilityCallbacks *callback) {
	Mutex::Autolock _l(mLock);		Mutex::Autolock _l(mLock);
			drainPendingCallbacksLocked();
	Callback cb(callback);		Callback cb(callback);
	mCallbacks.erase(cb);		mCallbacks.erase(cb);
	}		}
	@@ -329,10 +331,30 @@ void CameraManagerGlobal::registerExtendedAvailabilityCallback(
	void CameraManagerGlobal::unregisterExtendedAvailabilityCallback(		void CameraManagerGlobal::unregisterExtendedAvailabilityCallback(
	const ACameraManager_ExtendedAvailabilityCallbacks *callback) {		const ACameraManager_ExtendedAvailabilityCallbacks *callback) {
	Mutex::Autolock _l(mLock);		Mutex::Autolock _l(mLock);
			drainPendingCallbacksLocked();
	Callback cb(callback);		Callback cb(callback);
	mCallbacks.erase(cb);		mCallbacks.erase(cb);
	}		}

			void CameraManagerGlobal::onCallbackCalled() {
			Mutex::Autolock _l(mLock);
			if (mPendingCallbackCnt > 0) {
			mPendingCallbackCnt--;
			}
			mCallbacksCond.signal();
			}

			void CameraManagerGlobal::drainPendingCallbacksLocked() {
			while (mPendingCallbackCnt > 0) {
			auto res = mCallbacksCond.waitRelative(mLock, kCallbackDrainTimeout);
			if (res != NO_ERROR) {
			ALOGE("%s: Error waiting to drain callbacks: %s(%d)",
			__FUNCTION__, strerror(-res), res);
			break;
			}
			}
			}

	template <class T>		template <class T>
	void CameraManagerGlobal::registerAvailCallback(const T *callback) {		void CameraManagerGlobal::registerAvailCallback(const T *callback) {
	Mutex::Autolock _l(mLock);		Mutex::Autolock _l(mLock);
	@@ -351,6 +373,7 @@ void CameraManagerGlobal::registerAvailCallback(const T *callback) {
	msg->setPointer(kCallbackFpKey, (void *) cbFunc);		msg->setPointer(kCallbackFpKey, (void *) cbFunc);
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId.c_str()));		msg->setString(kCameraIdKey, AString(cameraId.c_str()));
			mPendingCallbackCnt++;
	msg->post();		msg->post();

	// Physical camera unavailable callback		// Physical camera unavailable callback
	@@ -363,6 +386,7 @@ void CameraManagerGlobal::registerAvailCallback(const T *callback) {
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId.c_str()));		msg->setString(kCameraIdKey, AString(cameraId.c_str()));
	msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId.c_str()));		msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId.c_str()));
			mPendingCallbackCnt++;
	msg->post();		msg->post();
	}		}
	}		}
	@@ -407,6 +431,15 @@ bool CameraManagerGlobal::isStatusAvailable(CameraDeviceStatus status) {

	void CameraManagerGlobal::CallbackHandler::onMessageReceived(		void CameraManagerGlobal::CallbackHandler::onMessageReceived(
	const sp<AMessage> &msg) {		const sp<AMessage> &msg) {
			onMessageReceivedInternal(msg);
			if (msg->what() == kWhatSendSingleCallback \|\|
			msg->what() == kWhatSendSinglePhysicalCameraCallback) {
			notifyParent();
			}
			}

			void CameraManagerGlobal::CallbackHandler::onMessageReceivedInternal(
			const sp<AMessage> &msg) {
	switch (msg->what()) {		switch (msg->what()) {
	case kWhatSendSingleCallback:		case kWhatSendSingleCallback:
	{		{
	@@ -466,6 +499,13 @@ void CameraManagerGlobal::CallbackHandler::onMessageReceived(
	}		}
	}		}

			void CameraManagerGlobal::CallbackHandler::notifyParent() {
			sp<CameraManagerGlobal> parent = mParent.promote();
			if (parent != nullptr) {
			parent->onCallbackCalled();
			}
			}

	hardware::Return<void> CameraManagerGlobal::CameraServiceListener::onStatusChanged(		hardware::Return<void> CameraManagerGlobal::CameraServiceListener::onStatusChanged(
	const CameraStatusAndId &statusAndId) {		const CameraStatusAndId &statusAndId) {
	sp<CameraManagerGlobal> cm = mCameraManager.promote();		sp<CameraManagerGlobal> cm = mCameraManager.promote();
	@@ -512,6 +552,7 @@ void CameraManagerGlobal::onStatusChangedLocked(
	msg->setPointer(kCallbackFpKey, (void *) cbFp);		msg->setPointer(kCallbackFpKey, (void *) cbFp);
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId.c_str()));		msg->setString(kCameraIdKey, AString(cameraId.c_str()));
			mPendingCallbackCnt++;
	msg->post();		msg->post();
	}		}
	if (status == CameraDeviceStatus::STATUS_NOT_PRESENT) {		if (status == CameraDeviceStatus::STATUS_NOT_PRESENT) {
	@@ -577,6 +618,7 @@ void CameraManagerGlobal::onStatusChangedLocked(
	msg->setPointer(kContextKey, cb.mContext);		msg->setPointer(kContextKey, cb.mContext);
	msg->setString(kCameraIdKey, AString(cameraId.c_str()));		msg->setString(kCameraIdKey, AString(cameraId.c_str()));
	msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId.c_str()));		msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId.c_str()));
			mPendingCallbackCnt++;
	msg->post();		msg->post();
	}		}
	}		}

camera/ndk/ndk_vendor/impl/ACameraManager.h

+12 −1

Original line number	Original line	Diff line number	Diff line
	@@ -156,6 +156,12 @@ class CameraManagerGlobal final : public RefBase {
	ACameraManager_PhysicalCameraAvailabilityCallback mPhysicalCamUnavailable;		ACameraManager_PhysicalCameraAvailabilityCallback mPhysicalCamUnavailable;
	void* mContext;		void* mContext;
	};		};

			android::Condition mCallbacksCond;
			size_t mPendingCallbackCnt = 0;
			void onCallbackCalled();
			void drainPendingCallbacksLocked();

	std::set<Callback> mCallbacks;		std::set<Callback> mCallbacks;

	// definition of handler and message		// definition of handler and message
	@@ -167,10 +173,15 @@ class CameraManagerGlobal final : public RefBase {
	static const char* kPhysicalCameraIdKey;		static const char* kPhysicalCameraIdKey;
	static const char* kCallbackFpKey;		static const char* kCallbackFpKey;
	static const char* kContextKey;		static const char* kContextKey;
			static const nsecs_t kCallbackDrainTimeout;
	class CallbackHandler : public AHandler {		class CallbackHandler : public AHandler {
	public:		public:
	CallbackHandler() {}		CallbackHandler(wp<CameraManagerGlobal> parent) : mParent(parent) {}
	void onMessageReceived(const sp<AMessage> &msg) override;		void onMessageReceived(const sp<AMessage> &msg) override;
			private:
			wp<CameraManagerGlobal> mParent;
			void notifyParent();
			void onMessageReceivedInternal(const sp<AMessage> &msg);
	};		};
	sp<CallbackHandler> mHandler;		sp<CallbackHandler> mHandler;
	sp<ALooper> mCbLooper; // Looper thread where callbacks actually happen on		sp<ALooper> mCbLooper; // Looper thread where callbacks actually happen on