Camera: Track provider instances
In some very rare corner cases where the binder death notification
gets delayed and races against the new provider 'onRegistration'
callback, camera service can temporarily hold two camera providers
with the same name. Depending on scheduling, if 'onRegistration'
executes first, it will get rejected as the provider name already
exists. The subsequent binder death notification callback will
remove all camera devices.
To avoid this, track all camera providers with a combination of the
provider name and unique instance id. If we receive a registration
callback with provider name that already exists, defer the
initialization sequence until the previous instance gets released
correctly.
Bug: 164019313
Test: cameraservice_test
--gtest_filter=CameraProviderManagerTest.BinderDeathRegistrationRaceTest,
Camera CTS,
Partner testing
Change-Id: I207a12da1fb09b1c45dae8697049f1fec34627d3
diff --git a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
index 855b5ab..a74fd9d 100644
--- a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
+++ b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
@@ -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;
@@ -173,6 +175,25 @@
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,
@@ -567,3 +588,47 @@
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";
+}