Merge "Camera: Fix race between callback and unregisteration of callbacks" into rvc-dev am: f8673bf03d am: 85a78eb376 am: 5da38f9198
Change-Id: If957cf94d948fdb925e2002def8092a10ffd8cf6
diff --git a/camera/ndk/impl/ACameraManager.cpp b/camera/ndk/impl/ACameraManager.cpp
index 3a678be..419250c 100644
--- a/camera/ndk/impl/ACameraManager.cpp
+++ b/camera/ndk/impl/ACameraManager.cpp
@@ -35,6 +35,7 @@
const char* CameraManagerGlobal::kPhysicalCameraIdKey = "PhysicalCameraId";
const char* CameraManagerGlobal::kCallbackFpKey = "CallbackFp";
const char* CameraManagerGlobal::kContextKey = "CallbackContext";
+const nsecs_t CameraManagerGlobal::kCallbackDrainTimeout = 5000000; // 5 ms
Mutex CameraManagerGlobal::sLock;
CameraManagerGlobal* CameraManagerGlobal::sInstance = nullptr;
@@ -117,7 +118,7 @@
return nullptr;
}
if (mHandler == nullptr) {
- mHandler = new CallbackHandler();
+ mHandler = new CallbackHandler(this);
}
mCbLooper->registerHandler(mHandler);
}
@@ -211,6 +212,9 @@
void CameraManagerGlobal::unregisterExtendedAvailabilityCallback(
const ACameraManager_ExtendedAvailabilityCallbacks *callback) {
Mutex::Autolock _l(mLock);
+
+ drainPendingCallbacksLocked();
+
Callback cb(callback);
mCallbacks.erase(cb);
}
@@ -223,10 +227,32 @@
void CameraManagerGlobal::unregisterAvailabilityCallback(
const ACameraManager_AvailabilityCallbacks *callback) {
Mutex::Autolock _l(mLock);
+
+ drainPendingCallbacksLocked();
+
Callback cb(callback);
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>
void CameraManagerGlobal::registerAvailCallback(const T *callback) {
Mutex::Autolock _l(mLock);
@@ -250,6 +276,7 @@
msg->setPointer(kCallbackFpKey, (void *) cbFunc);
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId));
+ mPendingCallbackCnt++;
msg->post();
// Physical camera unavailable callback
@@ -263,6 +290,7 @@
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId));
msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId));
+ mPendingCallbackCnt++;
msg->post();
}
}
@@ -323,6 +351,16 @@
}
void CameraManagerGlobal::CallbackHandler::onMessageReceived(
+ 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()) {
case kWhatSendSingleCallback:
@@ -405,6 +443,13 @@
}
}
+void CameraManagerGlobal::CallbackHandler::notifyParent() {
+ sp<CameraManagerGlobal> parent = mParent.promote();
+ if (parent != nullptr) {
+ parent->onCallbackCalled();
+ }
+}
+
binder::Status CameraManagerGlobal::CameraServiceListener::onCameraAccessPrioritiesChanged() {
sp<CameraManagerGlobal> cm = mCameraManager.promote();
if (cm != nullptr) {
@@ -445,6 +490,7 @@
if (cbFp != nullptr) {
msg->setPointer(kCallbackFpKey, (void *) cbFp);
msg->setPointer(kContextKey, cb.mContext);
+ mPendingCallbackCnt++;
msg->post();
}
}
@@ -491,6 +537,7 @@
msg->setPointer(kCallbackFpKey, (void *) cbFp);
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId));
+ mPendingCallbackCnt++;
msg->post();
}
}
@@ -545,6 +592,7 @@
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId));
msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId));
+ mPendingCallbackCnt++;
msg->post();
}
}
diff --git a/camera/ndk/impl/ACameraManager.h b/camera/ndk/impl/ACameraManager.h
index 7fba188..ccbfaa9 100644
--- a/camera/ndk/impl/ACameraManager.h
+++ b/camera/ndk/impl/ACameraManager.h
@@ -163,6 +163,12 @@
ACameraManager_PhysicalCameraAvailabilityCallback mPhysicalCamUnavailable;
void* mContext;
};
+
+ android::Condition mCallbacksCond;
+ size_t mPendingCallbackCnt = 0;
+ void onCallbackCalled();
+ void drainPendingCallbacksLocked();
+
std::set<Callback> mCallbacks;
// definition of handler and message
@@ -175,10 +181,16 @@
static const char* kPhysicalCameraIdKey;
static const char* kCallbackFpKey;
static const char* kContextKey;
+ static const nsecs_t kCallbackDrainTimeout;
class CallbackHandler : public AHandler {
public:
- CallbackHandler() {}
+ CallbackHandler(wp<CameraManagerGlobal> parent) : mParent(parent) {}
void onMessageReceived(const sp<AMessage> &msg) override;
+
+ private:
+ wp<CameraManagerGlobal> mParent;
+ void notifyParent();
+ void onMessageReceivedInternal(const sp<AMessage> &msg);
};
sp<CallbackHandler> mHandler;
sp<ALooper> mCbLooper; // Looper thread where callbacks actually happen on
diff --git a/camera/ndk/include/camera/NdkCameraManager.h b/camera/ndk/include/camera/NdkCameraManager.h
index e2b71bf..0a2ee57 100644
--- a/camera/ndk/include/camera/NdkCameraManager.h
+++ b/camera/ndk/include/camera/NdkCameraManager.h
@@ -191,6 +191,9 @@
*
* <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 callback the {@link ACameraManager_AvailabilityCallbacks} to be unregistered.
*
@@ -382,6 +385,9 @@
*
* <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 callback the {@link ACameraManager_ExtendedAvailabilityCallbacks} to be unregistered.
*
diff --git a/camera/ndk/ndk_vendor/impl/ACameraManager.cpp b/camera/ndk/ndk_vendor/impl/ACameraManager.cpp
index e2097b5..5aa9c46 100644
--- a/camera/ndk/ndk_vendor/impl/ACameraManager.cpp
+++ b/camera/ndk/ndk_vendor/impl/ACameraManager.cpp
@@ -45,6 +45,7 @@
const char* CameraManagerGlobal::kPhysicalCameraIdKey = "PhysicalCameraId";
const char* CameraManagerGlobal::kCallbackFpKey = "CallbackFp";
const char* CameraManagerGlobal::kContextKey = "CallbackContext";
+const nsecs_t CameraManagerGlobal::kCallbackDrainTimeout = 5000000; // 5 ms
Mutex CameraManagerGlobal::sLock;
CameraManagerGlobal* CameraManagerGlobal::sInstance = nullptr;
@@ -249,7 +250,7 @@
return nullptr;
}
if (mHandler == nullptr) {
- mHandler = new CallbackHandler();
+ mHandler = new CallbackHandler(this);
}
mCbLooper->registerHandler(mHandler);
}
@@ -317,6 +318,7 @@
void CameraManagerGlobal::unregisterAvailabilityCallback(
const ACameraManager_AvailabilityCallbacks *callback) {
Mutex::Autolock _l(mLock);
+ drainPendingCallbacksLocked();
Callback cb(callback);
mCallbacks.erase(cb);
}
@@ -329,10 +331,30 @@
void CameraManagerGlobal::unregisterExtendedAvailabilityCallback(
const ACameraManager_ExtendedAvailabilityCallbacks *callback) {
Mutex::Autolock _l(mLock);
+ drainPendingCallbacksLocked();
Callback cb(callback);
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>
void CameraManagerGlobal::registerAvailCallback(const T *callback) {
Mutex::Autolock _l(mLock);
@@ -351,6 +373,7 @@
msg->setPointer(kCallbackFpKey, (void *) cbFunc);
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId.c_str()));
+ mPendingCallbackCnt++;
msg->post();
// Physical camera unavailable callback
@@ -363,6 +386,7 @@
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId.c_str()));
msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId.c_str()));
+ mPendingCallbackCnt++;
msg->post();
}
}
@@ -406,6 +430,15 @@
}
void CameraManagerGlobal::CallbackHandler::onMessageReceived(
+ 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()) {
case kWhatSendSingleCallback:
@@ -466,6 +499,13 @@
}
}
+void CameraManagerGlobal::CallbackHandler::notifyParent() {
+ sp<CameraManagerGlobal> parent = mParent.promote();
+ if (parent != nullptr) {
+ parent->onCallbackCalled();
+ }
+}
+
hardware::Return<void> CameraManagerGlobal::CameraServiceListener::onStatusChanged(
const CameraStatusAndId &statusAndId) {
sp<CameraManagerGlobal> cm = mCameraManager.promote();
@@ -512,6 +552,7 @@
msg->setPointer(kCallbackFpKey, (void *) cbFp);
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId.c_str()));
+ mPendingCallbackCnt++;
msg->post();
}
if (status == CameraDeviceStatus::STATUS_NOT_PRESENT) {
@@ -577,6 +618,7 @@
msg->setPointer(kContextKey, cb.mContext);
msg->setString(kCameraIdKey, AString(cameraId.c_str()));
msg->setString(kPhysicalCameraIdKey, AString(physicalCameraId.c_str()));
+ mPendingCallbackCnt++;
msg->post();
}
}
diff --git a/camera/ndk/ndk_vendor/impl/ACameraManager.h b/camera/ndk/ndk_vendor/impl/ACameraManager.h
index 36c8e2b..85da3e9 100644
--- a/camera/ndk/ndk_vendor/impl/ACameraManager.h
+++ b/camera/ndk/ndk_vendor/impl/ACameraManager.h
@@ -156,6 +156,12 @@
ACameraManager_PhysicalCameraAvailabilityCallback mPhysicalCamUnavailable;
void* mContext;
};
+
+ android::Condition mCallbacksCond;
+ size_t mPendingCallbackCnt = 0;
+ void onCallbackCalled();
+ void drainPendingCallbacksLocked();
+
std::set<Callback> mCallbacks;
// definition of handler and message
@@ -167,10 +173,15 @@
static const char* kPhysicalCameraIdKey;
static const char* kCallbackFpKey;
static const char* kContextKey;
+ static const nsecs_t kCallbackDrainTimeout;
class CallbackHandler : public AHandler {
public:
- CallbackHandler() {}
+ CallbackHandler(wp<CameraManagerGlobal> parent) : mParent(parent) {}
void onMessageReceived(const sp<AMessage> &msg) override;
+ private:
+ wp<CameraManagerGlobal> mParent;
+ void notifyParent();
+ void onMessageReceivedInternal(const sp<AMessage> &msg);
};
sp<CallbackHandler> mHandler;
sp<ALooper> mCbLooper; // Looper thread where callbacks actually happen on