Camera: Fix race condition for filterSPerfClassCharacteristics
1. enumerateProviders may be called from different threads: from
cameraserver startup, and from the provider's onRegistration.
When enumerateProviders calls filterSPerfClassCharacteristics,
mServiceLock needs to be held.
2. onRegistration can be called with preexisting set to TRUE. In that
case, do not call onNewProviderRegistered() to avoid unnecessary
enumerateProviders.
Test: vendor testing, camera CTS
Bug: 193796282
Change-Id: I596a047cb36a9926014de232a6639a32da4f214c
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 3deea6b..dc101ff 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -237,10 +237,16 @@
}
}
- //Derive primary rear/front cameras, and filter their charactierstics.
- //This needs to be done after all cameras are enumerated and camera ids are sorted.
+ // Derive primary rear/front cameras, and filter their charactierstics.
+ // This needs to be done after all cameras are enumerated and camera ids are sorted.
if (SessionConfigurationUtils::IS_PERF_CLASS) {
- filterSPerfClassCharacteristics();
+ // Assume internal cameras are advertised from the same
+ // provider. If multiple providers are registered at different time,
+ // and each provider contains multiple internal color cameras, the current
+ // logic may filter the characteristics of more than one front/rear color
+ // cameras.
+ Mutex::Autolock l(mServiceLock);
+ filterSPerfClassCharacteristicsLocked();
}
return OK;
@@ -313,7 +319,7 @@
filterAPI1SystemCameraLocked(mNormalDeviceIds);
}
-void CameraService::filterSPerfClassCharacteristics() {
+void CameraService::filterSPerfClassCharacteristicsLocked() {
// To claim to be S Performance primary cameras, the cameras must be
// backward compatible. So performance class primary camera Ids must be API1
// compatible.
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index 1fb7104..9021170 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -945,9 +945,10 @@
void updateCameraNumAndIds();
/**
- * Filter camera characteristics for S Performance class primary cameras
+ * Filter camera characteristics for S Performance class primary cameras.
+ * mServiceLock should be locked.
*/
- void filterSPerfClassCharacteristics();
+ void filterSPerfClassCharacteristicsLocked();
// File descriptor to temp file used for caching previous open
// session dumpsys info.
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index 7045128..4f2b878 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -476,15 +476,16 @@
const hardware::hidl_string& /*fqName*/,
const hardware::hidl_string& name,
bool preexisting) {
+ status_t res = OK;
std::lock_guard<std::mutex> providerLock(mProviderLifecycleLock);
{
std::lock_guard<std::mutex> lock(mInterfaceMutex);
- addProviderLocked(name, preexisting);
+ res = addProviderLocked(name, preexisting);
}
sp<StatusListener> listener = getStatusListener();
- if (nullptr != listener.get()) {
+ if (nullptr != listener.get() && res == OK) {
listener->onNewProviderRegistered();
}