From 53704ac3f406a776cfe636c45fd7fdd305962788 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Sun, 18 Aug 2019 03:24:56 +0300
Subject: libcamera: camera_manager: Construct CameraManager instances manually

The CameraManager class is not supposed to be instantiated multiple
times, which led to a singleton implementation. This requires a global
instance of the CameraManager, which is destroyed when the global
destructors are executed.

Relying on global instances causes issues with cleanup, as the order in
which the global destructors are run can't be controlled. In particular,
the Android camera HAL implementation ends up destroying the
CameraHalManager after the CameraManager, which leads to use-after-free
problems.

To solve this, remove the CameraManager::instance() method and make the
CameraManager class instantiable directly. Multiple instances are still
not allowed, and this is enforced by storing the instance pointer
internally to be checked when an instance is created.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 test/camera/camera_test.cpp               |  3 ++-
 test/controls/control_list.cpp            |  3 ++-
 test/list-cameras.cpp                     | 11 ++++++-----
 test/pipeline/ipu3/ipu3_pipeline_test.cpp |  3 ++-
 4 files changed, 12 insertions(+), 8 deletions(-)

(limited to 'test')

diff --git a/test/camera/camera_test.cpp b/test/camera/camera_test.cpp
index 24ff5fe0..0e105414 100644
--- a/test/camera/camera_test.cpp
+++ b/test/camera/camera_test.cpp
@@ -14,7 +14,7 @@ using namespace std;
 
 int CameraTest::init()
 {
-	cm_ = CameraManager::instance();
+	cm_ = new CameraManager();
 
 	if (cm_->start()) {
 		cout << "Failed to start camera manager" << endl;
@@ -44,4 +44,5 @@ void CameraTest::cleanup()
 	}
 
 	cm_->stop();
+	delete cm_;
 };
diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index c834edc3..f1d79ff8 100644
--- a/test/controls/control_list.cpp
+++ b/test/controls/control_list.cpp
@@ -21,7 +21,7 @@ class ControlListTest : public Test
 protected:
 	int init()
 	{
-		cm_ = CameraManager::instance();
+		cm_ = new CameraManager();
 
 		if (cm_->start()) {
 			cout << "Failed to start camera manager" << endl;
@@ -203,6 +203,7 @@ protected:
 		}
 
 		cm_->stop();
+		delete cm_;
 	}
 
 private:
diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp
index 070cbf2b..55551d7e 100644
--- a/test/list-cameras.cpp
+++ b/test/list-cameras.cpp
@@ -20,8 +20,8 @@ class ListTest : public Test
 protected:
 	int init()
 	{
-		cm = CameraManager::instance();
-		cm->start();
+		cm_ = new CameraManager();
+		cm_->start();
 
 		return 0;
 	}
@@ -30,7 +30,7 @@ protected:
 	{
 		unsigned int count = 0;
 
-		for (const std::shared_ptr<Camera> &camera : cm->cameras()) {
+		for (const std::shared_ptr<Camera> &camera : cm_->cameras()) {
 			cout << "- " << camera->name() << endl;
 			count++;
 		}
@@ -40,11 +40,12 @@ protected:
 
 	void cleanup()
 	{
-		cm->stop();
+		cm_->stop();
+		delete cm_;
 	}
 
 private:
-	CameraManager *cm;
+	CameraManager *cm_;
 };
 
 TEST_REGISTER(ListTest)
diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
index 1d4cc4d4..8bfcd609 100644
--- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp
+++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
@@ -92,7 +92,7 @@ int IPU3PipelineTest::init()
 
 	enumerator.reset(nullptr);
 
-	cameraManager_ = CameraManager::instance();
+	cameraManager_ = new CameraManager();
 	ret = cameraManager_->start();
 	if (ret) {
 		cerr << "Failed to start the CameraManager" << endl;
@@ -120,6 +120,7 @@ int IPU3PipelineTest::run()
 void IPU3PipelineTest::cleanup()
 {
 	cameraManager_->stop();
+	delete cameraManager_;
 }
 
 TEST_REGISTER(IPU3PipelineTest)
-- 
cgit v1.2.1