Loading services/camera/libcameraservice/common/CameraProviderManager.cpp +55 −21 Original line number Diff line number Diff line Loading @@ -474,12 +474,12 @@ void CameraProviderManager::removeRef(DeviceMode usageType, const std::string &c hardware::Return<void> CameraProviderManager::onRegistration( const hardware::hidl_string& /*fqName*/, const hardware::hidl_string& name, bool /*preexisting*/) { bool preexisting) { std::lock_guard<std::mutex> providerLock(mProviderLifecycleLock); { std::lock_guard<std::mutex> lock(mInterfaceMutex); addProviderLocked(name); addProviderLocked(name, preexisting); } sp<StatusListener> listener = getStatusListener(); Loading Loading @@ -1230,33 +1230,53 @@ CameraProviderManager::isHiddenPhysicalCameraInternal(const std::string& cameraI return falseRet; } status_t CameraProviderManager::addProviderLocked(const std::string& newProvider) { for (const auto& providerInfo : mProviders) { if (providerInfo->mProviderName == newProvider) { ALOGW("%s: Camera provider HAL with name '%s' already registered", __FUNCTION__, newProvider.c_str()); return ALREADY_EXISTS; } } status_t CameraProviderManager::tryToInitializeProviderLocked( const std::string& providerName, const sp<ProviderInfo>& providerInfo) { sp<provider::V2_4::ICameraProvider> interface; interface = mServiceProxy->tryGetService(newProvider); interface = mServiceProxy->tryGetService(providerName); if (interface == nullptr) { // The interface may not be started yet. In that case, this is not a // fatal error. ALOGW("%s: Camera provider HAL '%s' is not actually available", __FUNCTION__, newProvider.c_str()); providerName.c_str()); return BAD_VALUE; } sp<ProviderInfo> providerInfo = new ProviderInfo(newProvider, this); status_t res = providerInfo->initialize(interface, mDeviceState); return providerInfo->initialize(interface, mDeviceState); } status_t CameraProviderManager::addProviderLocked(const std::string& newProvider, bool preexisting) { // Several camera provider instances can be temporarily present. // Defer initialization of a new instance until the older instance is properly removed. auto providerInstance = newProvider + "-" + std::to_string(mProviderInstanceId); bool providerPresent = false; for (const auto& providerInfo : mProviders) { if (providerInfo->mProviderName == newProvider) { ALOGW("%s: Camera provider HAL with name '%s' already registered", __FUNCTION__, newProvider.c_str()); if (preexisting) { return ALREADY_EXISTS; } else{ ALOGW("%s: The new provider instance will get initialized immediately after the" " currently present instance is removed!", __FUNCTION__); providerPresent = true; break; } } } sp<ProviderInfo> providerInfo = new ProviderInfo(newProvider, providerInstance, this); if (!providerPresent) { status_t res = tryToInitializeProviderLocked(newProvider, providerInfo); if (res != OK) { return res; } } mProviders.push_back(providerInfo); mProviderInstanceId++; return OK; } Loading @@ -1266,12 +1286,14 @@ status_t CameraProviderManager::removeProvider(const std::string& provider) { std::unique_lock<std::mutex> lock(mInterfaceMutex); std::vector<String8> removedDeviceIds; status_t res = NAME_NOT_FOUND; std::string removedProviderName; for (auto it = mProviders.begin(); it != mProviders.end(); it++) { if ((*it)->mProviderName == provider) { if ((*it)->mProviderInstance == provider) { removedDeviceIds.reserve((*it)->mDevices.size()); for (auto& deviceInfo : (*it)->mDevices) { removedDeviceIds.push_back(String8(deviceInfo->mId.c_str())); } removedProviderName = (*it)->mProviderName; mProviders.erase(it); res = OK; break; Loading @@ -1281,6 +1303,14 @@ status_t CameraProviderManager::removeProvider(const std::string& provider) { ALOGW("%s: Camera provider HAL with name '%s' is not registered", __FUNCTION__, provider.c_str()); } else { // Check if there are any newer camera instances from the same provider and try to // initialize. for (const auto& providerInfo : mProviders) { if (providerInfo->mProviderName == removedProviderName) { return tryToInitializeProviderLocked(removedProviderName, providerInfo); } } // Inform camera service of loss of presence for all the devices from this provider, // without lock held for reentrancy sp<StatusListener> listener = getStatusListener(); Loading @@ -1289,7 +1319,9 @@ status_t CameraProviderManager::removeProvider(const std::string& provider) { for (auto& id : removedDeviceIds) { listener->onDeviceStatusChanged(id, CameraDeviceStatus::NOT_PRESENT); } lock.lock(); } } return res; } Loading @@ -1303,8 +1335,10 @@ sp<CameraProviderManager::StatusListener> CameraProviderManager::getStatusListen CameraProviderManager::ProviderInfo::ProviderInfo( const std::string &providerName, const std::string &providerInstance, CameraProviderManager *manager) : mProviderName(providerName), mProviderInstance(providerInstance), mProviderTagid(generateVendorTagId(providerName)), mUniqueDeviceCount(0), mManager(manager) { Loading Loading @@ -1628,7 +1662,7 @@ void CameraProviderManager::ProviderInfo::removeDevice(std::string id) { status_t CameraProviderManager::ProviderInfo::dump(int fd, const Vector<String16>&) const { dprintf(fd, "== Camera Provider HAL %s (v2.%d, %s) static info: %zu devices: ==\n", mProviderName.c_str(), mProviderInstance.c_str(), mMinorVersion, mIsRemote ? "remote" : "passthrough", mDevices.size()); Loading Loading @@ -1944,12 +1978,12 @@ hardware::Return<void> CameraProviderManager::ProviderInfo::torchModeStatusChang void CameraProviderManager::ProviderInfo::serviceDied(uint64_t cookie, const wp<hidl::base::V1_0::IBase>& who) { (void) who; ALOGI("Camera provider '%s' has died; removing it", mProviderName.c_str()); ALOGI("Camera provider '%s' has died; removing it", mProviderInstance.c_str()); if (cookie != mId) { ALOGW("%s: Unexpected serviceDied cookie %" PRIu64 ", expected %" PRIu32, __FUNCTION__, cookie, mId); } mManager->removeProvider(mProviderName); mManager->removeProvider(mProviderInstance); } status_t CameraProviderManager::ProviderInfo::setUpVendorTags() { Loading services/camera/libcameraservice/common/CameraProviderManager.h +7 −2 Original line number Diff line number Diff line Loading @@ -365,6 +365,7 @@ private: virtual public hardware::hidl_death_recipient { const std::string mProviderName; const std::string mProviderInstance; const metadata_vendor_id_t mProviderTagid; int mMinorVersion; sp<VendorTagDescriptor> mVendorTagDescriptor; Loading @@ -379,7 +380,7 @@ private: sp<hardware::camera::provider::V2_4::ICameraProvider> mSavedInterface; ProviderInfo(const std::string &providerName, ProviderInfo(const std::string &providerName, const std::string &providerInstance, CameraProviderManager *manager); ~ProviderInfo(); Loading Loading @@ -657,7 +658,10 @@ private: hardware::hidl_version minVersion = hardware::hidl_version{0,0}, hardware::hidl_version maxVersion = hardware::hidl_version{1000,0}) const; status_t addProviderLocked(const std::string& newProvider); status_t addProviderLocked(const std::string& newProvider, bool preexisting = false); status_t tryToInitializeProviderLocked(const std::string& providerName, const sp<ProviderInfo>& providerInfo); bool isLogicalCameraLocked(const std::string& id, std::vector<std::string>* physicalCameraIds); Loading @@ -666,6 +670,7 @@ private: bool isValidDeviceLocked(const std::string &id, uint16_t majorVersion) const; size_t mProviderInstanceId = 0; std::vector<sp<ProviderInfo>> mProviders; void addProviderToMap( Loading services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp +65 −0 Original line number Diff line number Diff line Loading @@ -23,7 +23,9 @@ #include <android/hardware/camera/device/3.2/ICameraDeviceCallback.h> #include <android/hardware/camera/device/3.2/ICameraDeviceSession.h> #include <camera_metadata_hidden.h> #include <hidl/HidlBinderSupport.h> #include <gtest/gtest.h> #include <utility> using namespace android; using namespace android::hardware::camera; Loading Loading @@ -173,6 +175,25 @@ struct TestICameraProvider : virtual public provider::V2_5::ICameraProvider { return hardware::Void(); } virtual ::android::hardware::Return<bool> linkToDeath( const ::android::sp<::android::hardware::hidl_death_recipient>& recipient, uint64_t cookie) { if (mInitialDeathRecipient.get() == nullptr) { mInitialDeathRecipient = std::make_unique<::android::hardware::hidl_binder_death_recipient>(recipient, cookie, this); } return true; } void signalInitialBinderDeathRecipient() { if (mInitialDeathRecipient.get() != nullptr) { mInitialDeathRecipient->binderDied(nullptr /*who*/); } } std::unique_ptr<::android::hardware::hidl_binder_death_recipient> mInitialDeathRecipient; enum MethodNames { SET_CALLBACK, GET_VENDOR_TAGS, Loading Loading @@ -567,3 +588,47 @@ TEST(CameraProviderManagerTest, BadHalStartupTest) { ASSERT_EQ(serviceProxy.mLastRequestedServiceNames.back(), testProviderInstanceName) << "Incorrect instance requested from service manager"; } // Test that CameraProviderManager can handle races between provider death notifications and // provider registration callbacks TEST(CameraProviderManagerTest, BinderDeathRegistrationRaceTest) { std::vector<hardware::hidl_string> deviceNames; deviceNames.push_back("device@3.2/test/0"); deviceNames.push_back("device@3.2/test/1"); hardware::hidl_vec<common::V1_0::VendorTagSection> vendorSection; status_t res; sp<CameraProviderManager> providerManager = new CameraProviderManager(); sp<TestStatusListener> statusListener = new TestStatusListener(); TestInteractionProxy serviceProxy; sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames, vendorSection); // Not setting up provider in the service proxy yet, to test cases where a // HAL isn't starting right res = providerManager->initialize(statusListener, &serviceProxy); ASSERT_EQ(res, OK) << "Unable to initialize provider manager"; // Now set up provider and trigger a registration serviceProxy.setProvider(provider); hardware::hidl_string testProviderFqInterfaceName = "android.hardware.camera.provider@2.4::ICameraProvider"; hardware::hidl_string testProviderInstanceName = "test/0"; serviceProxy.mManagerNotificationInterface->onRegistration( testProviderFqInterfaceName, testProviderInstanceName, false); // Simulate artificial delay of the registration callback which arrives before the // death notification serviceProxy.mManagerNotificationInterface->onRegistration( testProviderFqInterfaceName, testProviderInstanceName, false); provider->signalInitialBinderDeathRecipient(); auto deviceCount = static_cast<unsigned> (providerManager->getCameraCount().second); ASSERT_EQ(deviceCount, deviceNames.size()) << "Unexpected amount of camera devices"; } Loading
services/camera/libcameraservice/common/CameraProviderManager.cpp +55 −21 Original line number Diff line number Diff line Loading @@ -474,12 +474,12 @@ void CameraProviderManager::removeRef(DeviceMode usageType, const std::string &c hardware::Return<void> CameraProviderManager::onRegistration( const hardware::hidl_string& /*fqName*/, const hardware::hidl_string& name, bool /*preexisting*/) { bool preexisting) { std::lock_guard<std::mutex> providerLock(mProviderLifecycleLock); { std::lock_guard<std::mutex> lock(mInterfaceMutex); addProviderLocked(name); addProviderLocked(name, preexisting); } sp<StatusListener> listener = getStatusListener(); Loading Loading @@ -1230,33 +1230,53 @@ CameraProviderManager::isHiddenPhysicalCameraInternal(const std::string& cameraI return falseRet; } status_t CameraProviderManager::addProviderLocked(const std::string& newProvider) { for (const auto& providerInfo : mProviders) { if (providerInfo->mProviderName == newProvider) { ALOGW("%s: Camera provider HAL with name '%s' already registered", __FUNCTION__, newProvider.c_str()); return ALREADY_EXISTS; } } status_t CameraProviderManager::tryToInitializeProviderLocked( const std::string& providerName, const sp<ProviderInfo>& providerInfo) { sp<provider::V2_4::ICameraProvider> interface; interface = mServiceProxy->tryGetService(newProvider); interface = mServiceProxy->tryGetService(providerName); if (interface == nullptr) { // The interface may not be started yet. In that case, this is not a // fatal error. ALOGW("%s: Camera provider HAL '%s' is not actually available", __FUNCTION__, newProvider.c_str()); providerName.c_str()); return BAD_VALUE; } sp<ProviderInfo> providerInfo = new ProviderInfo(newProvider, this); status_t res = providerInfo->initialize(interface, mDeviceState); return providerInfo->initialize(interface, mDeviceState); } status_t CameraProviderManager::addProviderLocked(const std::string& newProvider, bool preexisting) { // Several camera provider instances can be temporarily present. // Defer initialization of a new instance until the older instance is properly removed. auto providerInstance = newProvider + "-" + std::to_string(mProviderInstanceId); bool providerPresent = false; for (const auto& providerInfo : mProviders) { if (providerInfo->mProviderName == newProvider) { ALOGW("%s: Camera provider HAL with name '%s' already registered", __FUNCTION__, newProvider.c_str()); if (preexisting) { return ALREADY_EXISTS; } else{ ALOGW("%s: The new provider instance will get initialized immediately after the" " currently present instance is removed!", __FUNCTION__); providerPresent = true; break; } } } sp<ProviderInfo> providerInfo = new ProviderInfo(newProvider, providerInstance, this); if (!providerPresent) { status_t res = tryToInitializeProviderLocked(newProvider, providerInfo); if (res != OK) { return res; } } mProviders.push_back(providerInfo); mProviderInstanceId++; return OK; } Loading @@ -1266,12 +1286,14 @@ status_t CameraProviderManager::removeProvider(const std::string& provider) { std::unique_lock<std::mutex> lock(mInterfaceMutex); std::vector<String8> removedDeviceIds; status_t res = NAME_NOT_FOUND; std::string removedProviderName; for (auto it = mProviders.begin(); it != mProviders.end(); it++) { if ((*it)->mProviderName == provider) { if ((*it)->mProviderInstance == provider) { removedDeviceIds.reserve((*it)->mDevices.size()); for (auto& deviceInfo : (*it)->mDevices) { removedDeviceIds.push_back(String8(deviceInfo->mId.c_str())); } removedProviderName = (*it)->mProviderName; mProviders.erase(it); res = OK; break; Loading @@ -1281,6 +1303,14 @@ status_t CameraProviderManager::removeProvider(const std::string& provider) { ALOGW("%s: Camera provider HAL with name '%s' is not registered", __FUNCTION__, provider.c_str()); } else { // Check if there are any newer camera instances from the same provider and try to // initialize. for (const auto& providerInfo : mProviders) { if (providerInfo->mProviderName == removedProviderName) { return tryToInitializeProviderLocked(removedProviderName, providerInfo); } } // Inform camera service of loss of presence for all the devices from this provider, // without lock held for reentrancy sp<StatusListener> listener = getStatusListener(); Loading @@ -1289,7 +1319,9 @@ status_t CameraProviderManager::removeProvider(const std::string& provider) { for (auto& id : removedDeviceIds) { listener->onDeviceStatusChanged(id, CameraDeviceStatus::NOT_PRESENT); } lock.lock(); } } return res; } Loading @@ -1303,8 +1335,10 @@ sp<CameraProviderManager::StatusListener> CameraProviderManager::getStatusListen CameraProviderManager::ProviderInfo::ProviderInfo( const std::string &providerName, const std::string &providerInstance, CameraProviderManager *manager) : mProviderName(providerName), mProviderInstance(providerInstance), mProviderTagid(generateVendorTagId(providerName)), mUniqueDeviceCount(0), mManager(manager) { Loading Loading @@ -1628,7 +1662,7 @@ void CameraProviderManager::ProviderInfo::removeDevice(std::string id) { status_t CameraProviderManager::ProviderInfo::dump(int fd, const Vector<String16>&) const { dprintf(fd, "== Camera Provider HAL %s (v2.%d, %s) static info: %zu devices: ==\n", mProviderName.c_str(), mProviderInstance.c_str(), mMinorVersion, mIsRemote ? "remote" : "passthrough", mDevices.size()); Loading Loading @@ -1944,12 +1978,12 @@ hardware::Return<void> CameraProviderManager::ProviderInfo::torchModeStatusChang void CameraProviderManager::ProviderInfo::serviceDied(uint64_t cookie, const wp<hidl::base::V1_0::IBase>& who) { (void) who; ALOGI("Camera provider '%s' has died; removing it", mProviderName.c_str()); ALOGI("Camera provider '%s' has died; removing it", mProviderInstance.c_str()); if (cookie != mId) { ALOGW("%s: Unexpected serviceDied cookie %" PRIu64 ", expected %" PRIu32, __FUNCTION__, cookie, mId); } mManager->removeProvider(mProviderName); mManager->removeProvider(mProviderInstance); } status_t CameraProviderManager::ProviderInfo::setUpVendorTags() { Loading
services/camera/libcameraservice/common/CameraProviderManager.h +7 −2 Original line number Diff line number Diff line Loading @@ -365,6 +365,7 @@ private: virtual public hardware::hidl_death_recipient { const std::string mProviderName; const std::string mProviderInstance; const metadata_vendor_id_t mProviderTagid; int mMinorVersion; sp<VendorTagDescriptor> mVendorTagDescriptor; Loading @@ -379,7 +380,7 @@ private: sp<hardware::camera::provider::V2_4::ICameraProvider> mSavedInterface; ProviderInfo(const std::string &providerName, ProviderInfo(const std::string &providerName, const std::string &providerInstance, CameraProviderManager *manager); ~ProviderInfo(); Loading Loading @@ -657,7 +658,10 @@ private: hardware::hidl_version minVersion = hardware::hidl_version{0,0}, hardware::hidl_version maxVersion = hardware::hidl_version{1000,0}) const; status_t addProviderLocked(const std::string& newProvider); status_t addProviderLocked(const std::string& newProvider, bool preexisting = false); status_t tryToInitializeProviderLocked(const std::string& providerName, const sp<ProviderInfo>& providerInfo); bool isLogicalCameraLocked(const std::string& id, std::vector<std::string>* physicalCameraIds); Loading @@ -666,6 +670,7 @@ private: bool isValidDeviceLocked(const std::string &id, uint16_t majorVersion) const; size_t mProviderInstanceId = 0; std::vector<sp<ProviderInfo>> mProviders; void addProviderToMap( Loading
services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp +65 −0 Original line number Diff line number Diff line Loading @@ -23,7 +23,9 @@ #include <android/hardware/camera/device/3.2/ICameraDeviceCallback.h> #include <android/hardware/camera/device/3.2/ICameraDeviceSession.h> #include <camera_metadata_hidden.h> #include <hidl/HidlBinderSupport.h> #include <gtest/gtest.h> #include <utility> using namespace android; using namespace android::hardware::camera; Loading Loading @@ -173,6 +175,25 @@ struct TestICameraProvider : virtual public provider::V2_5::ICameraProvider { return hardware::Void(); } virtual ::android::hardware::Return<bool> linkToDeath( const ::android::sp<::android::hardware::hidl_death_recipient>& recipient, uint64_t cookie) { if (mInitialDeathRecipient.get() == nullptr) { mInitialDeathRecipient = std::make_unique<::android::hardware::hidl_binder_death_recipient>(recipient, cookie, this); } return true; } void signalInitialBinderDeathRecipient() { if (mInitialDeathRecipient.get() != nullptr) { mInitialDeathRecipient->binderDied(nullptr /*who*/); } } std::unique_ptr<::android::hardware::hidl_binder_death_recipient> mInitialDeathRecipient; enum MethodNames { SET_CALLBACK, GET_VENDOR_TAGS, Loading Loading @@ -567,3 +588,47 @@ TEST(CameraProviderManagerTest, BadHalStartupTest) { ASSERT_EQ(serviceProxy.mLastRequestedServiceNames.back(), testProviderInstanceName) << "Incorrect instance requested from service manager"; } // Test that CameraProviderManager can handle races between provider death notifications and // provider registration callbacks TEST(CameraProviderManagerTest, BinderDeathRegistrationRaceTest) { std::vector<hardware::hidl_string> deviceNames; deviceNames.push_back("device@3.2/test/0"); deviceNames.push_back("device@3.2/test/1"); hardware::hidl_vec<common::V1_0::VendorTagSection> vendorSection; status_t res; sp<CameraProviderManager> providerManager = new CameraProviderManager(); sp<TestStatusListener> statusListener = new TestStatusListener(); TestInteractionProxy serviceProxy; sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames, vendorSection); // Not setting up provider in the service proxy yet, to test cases where a // HAL isn't starting right res = providerManager->initialize(statusListener, &serviceProxy); ASSERT_EQ(res, OK) << "Unable to initialize provider manager"; // Now set up provider and trigger a registration serviceProxy.setProvider(provider); hardware::hidl_string testProviderFqInterfaceName = "android.hardware.camera.provider@2.4::ICameraProvider"; hardware::hidl_string testProviderInstanceName = "test/0"; serviceProxy.mManagerNotificationInterface->onRegistration( testProviderFqInterfaceName, testProviderInstanceName, false); // Simulate artificial delay of the registration callback which arrives before the // death notification serviceProxy.mManagerNotificationInterface->onRegistration( testProviderFqInterfaceName, testProviderInstanceName, false); provider->signalInitialBinderDeathRecipient(); auto deviceCount = static_cast<unsigned> (providerManager->getCameraCount().second); ASSERT_EQ(deviceCount, deviceNames.size()) << "Unexpected amount of camera devices"; }