[libcamera-devel] [PATCH 1/5] IPAManager: make IPAManager lifetime explicitly managed

[Paul Elder]

If any ipa_context instances are destroyed before the IPAManager is
destroyed, then a segfault will occur when the IPAManager is
deconstructed, as it tries to destroy all of the IPAs that it manages.
Fix this by making the lifetime of the IPAManager explicit, and make the
CameraManager construct and deconstruct (automatically, via a unique
pointer) the IPAManager.

Also update the IPA interface test to do the construction and
deconstruction of the IPAManager, as it does not use the CameraManager.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
---
 include/libcamera/camera_manager.h       |  4 ++++
 include/libcamera/internal/ipa_manager.h |  7 ++++---
 src/libcamera/camera_manager.cpp         |  4 +++-
 src/libcamera/ipa_manager.cpp            | 13 +++++++++++--
 test/ipa/ipa_interface_test.cpp          |  5 +++++
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
index 079f848a..8f9398fa 100644
--- a/include/libcamera/camera_manager.h
+++ b/include/libcamera/camera_manager.h
@@ -14,6 +14,8 @@
 
 #include <libcamera/object.h>
 
+#include "libcamera/internal/ipa_manager.h"
+
 namespace libcamera {
 
 class Camera;
@@ -48,6 +50,8 @@ private:
 
 	class Private;
 	std::unique_ptr<Private> p_;
+
+	std::unique_ptr<IPAManager> ipaManager_;
 };
 
 } /* namespace libcamera */
diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
index 2412d757..65303d6d 100644
--- a/include/libcamera/internal/ipa_manager.h
+++ b/include/libcamera/internal/ipa_manager.h
@@ -22,6 +22,9 @@ namespace libcamera {
 class IPAManager
 {
 public:
+	IPAManager();
+	~IPAManager();
+
 	static IPAManager *instance();
 
 	std::unique_ptr<IPAProxy> createIPA(PipelineHandler *pipe,
@@ -30,9 +33,7 @@ public:
 
 private:
 	std::vector<IPAModule *> modules_;
-
-	IPAManager();
-	~IPAManager();
+	static IPAManager *self_;
 
 	void parseDir(const char *libDir, unsigned int maxDepth,
 		      std::vector<std::string> &files);
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 849377ad..d19348df 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -15,6 +15,7 @@
 
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/event_dispatcher_poll.h"
+#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/log.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/thread.h"
@@ -246,7 +247,8 @@ void CameraManager::Private::removeCamera(Camera *camera)
 CameraManager *CameraManager::self_ = nullptr;
 
 CameraManager::CameraManager()
-	: p_(new CameraManager::Private(this))
+	: p_(new CameraManager::Private(this)), ipaManager_(new IPAManager())
+
 {
 	if (self_)
 		LOG(Camera, Fatal)
diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index 505cf610..74112cde 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -93,8 +93,14 @@ LOG_DEFINE_CATEGORY(IPAManager)
  * plain C API, or to transmit the data to the isolated process through IPC.
  */
 
+IPAManager *IPAManager::self_ = nullptr;
+
 IPAManager::IPAManager()
 {
+	if (self_)
+		LOG(IPAManager, Fatal)
+			<< "Multiple IPAManager objects are not allowed";
+
 	unsigned int ipaCount = 0;
 
 	/* User-specified paths take precedence. */
@@ -134,12 +140,16 @@ IPAManager::IPAManager()
 	if (!ipaCount)
 		LOG(IPAManager, Warning)
 			<< "No IPA found in '" IPA_MODULE_DIR "'";
+
+	self_ = this;
 }
 
 IPAManager::~IPAManager()
 {
 	for (IPAModule *module : modules_)
 		delete module;
+
+	self_ = nullptr;
 }
 
 /**
@@ -153,8 +163,7 @@ IPAManager::~IPAManager()
  */
 IPAManager *IPAManager::instance()
 {
-	static IPAManager ipaManager;
-	return &ipaManager;
+	return self_;
 }
 
 /**
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index 2f02af49..153493ba 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -39,11 +39,15 @@ public:
 	~IPAInterfaceTest()
 	{
 		delete notifier_;
+		ipa_.reset();
+		ipaManager_.reset();
 	}
 
 protected:
 	int init() override
 	{
+		ipaManager_ = make_unique<IPAManager>();
+
 		/* Create a pipeline handler for vimc. */
 		std::vector<PipelineHandlerFactory *> &factories =
 			PipelineHandlerFactory::factories();
@@ -161,6 +165,7 @@ private:
 
 	std::shared_ptr<PipelineHandler> pipe_;
 	std::unique_ptr<IPAProxy> ipa_;
+	std::unique_ptr<IPAManager> ipaManager_;
 	enum IPAOperationCode trace_;
 	EventNotifier *notifier_;
 	int fd_;
-- 
2.20.1