libcamera: ProcessManager: make ProcessManager lifetime explicitly managed

If any Process instances are destroyed after the ProcessManager is
destroyed, then a segfault will occur.

Fix this by making the lifetime of the ProcessManager explicit, and make
the CameraManager construct and deconstruct (automatically, via a member
variable) the ProcessManager.

Update the tests accordingly.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

5 files changed, 54 insertions, 27 deletions

diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h
index 2688557c..254cda85 100644
--- a/include/libcamera/internal/process.h
+++ b/include/libcamera/internal/process.h
@@ -7,6 +7,7 @@
 #ifndef __LIBCAMERA_INTERNAL_PROCESS_H__
 #define __LIBCAMERA_INTERNAL_PROCESS_H__
 
+#include <signal.h>
 #include <string>
 #include <vector>
 
@@ -14,6 +15,8 @@
 
 namespace libcamera {
 
+class EventNotifier;
+
 class Process final
 {
 public:
@@ -50,6 +53,32 @@ private:
 	friend class ProcessManager;
 };
 
+class ProcessManager
+{
+public:
+	ProcessManager();
+	~ProcessManager();
+
+	void registerProcess(Process *proc);
+
+	static ProcessManager *instance();
+
+	int writePipe() const;
+
+	const struct sigaction &oldsa() const;
+
+private:
+	static ProcessManager *self_;
+
+	void sighandler(EventNotifier *notifier);
+
+	std::list<Process *> processes_;
+
+	struct sigaction oldsa_;
+	EventNotifier *sigEvent_;
+	int pipe_[2];
+};
+
 } /* namespace libcamera */
 
 #endif /* __LIBCAMERA_INTERNAL_PROCESS_H__ */
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 47d56256..b8d3ccc8 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -18,6 +18,7 @@
 #include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/log.h"
 #include "libcamera/internal/pipeline_handler.h"
+#include "libcamera/internal/process.h"
 #include "libcamera/internal/thread.h"
 #include "libcamera/internal/utils.h"
 
@@ -67,6 +68,7 @@ private:
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 
 	IPAManager ipaManager_;
+	ProcessManager processManager_;
 };
 
 CameraManager::Private::Private(CameraManager *cm)
diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
index 994190dc..72b5afe2 100644
--- a/src/libcamera/process.cpp
+++ b/src/libcamera/process.cpp
@@ -41,28 +41,6 @@ LOG_DEFINE_CATEGORY(Process)
  * The ProcessManager singleton keeps track of all created Process instances,
  * and manages the signal handling involved in terminating processes.
  */
-class ProcessManager
-{
-public:
-	void registerProcess(Process *proc);
-
-	static ProcessManager *instance();
-
-	int writePipe() const;
-
-	const struct sigaction &oldsa() const;
-
-private:
-	void sighandler(EventNotifier *notifier);
-	ProcessManager();
-	~ProcessManager();
-
-	std::list<Process *> processes_;
-
-	struct sigaction oldsa_;
-	EventNotifier *sigEvent_;
-	int pipe_[2];
-};
 
 namespace {
 
@@ -127,8 +105,20 @@ void ProcessManager::registerProcess(Process *proc)
 	processes_.push_back(proc);
 }
 
+ProcessManager *ProcessManager::self_ = nullptr;
+
+/**
+ * \brief Construct a ProcessManager instance
+ *
+ * The ProcessManager class is meant to only be instantiated once, by the
+ * CameraManager.
+ */
 ProcessManager::ProcessManager()
 {
+	if (self_)
+		LOG(Process, Fatal)
+			<< "Multiple ProcessManager objects are not allowed";
+
 	sigaction(SIGCHLD, NULL, &oldsa_);
 
 	struct sigaction sa;
@@ -145,6 +135,8 @@ ProcessManager::ProcessManager()
 			<< "Failed to initialize pipe for signal handling";
 	sigEvent_ = new EventNotifier(pipe_[0], EventNotifier::Read);
 	sigEvent_->activated.connect(this, &ProcessManager::sighandler);
+
+	self_ = this;
 }
 
 ProcessManager::~ProcessManager()
@@ -153,21 +145,21 @@ ProcessManager::~ProcessManager()
 	delete sigEvent_;
 	close(pipe_[0]);
 	close(pipe_[1]);
+
+	self_ = nullptr;
 }
 
 /**
  * \brief Retrieve the Process manager instance
  *
- * The ProcessManager is a singleton and can't be constructed manually. This
- * method shall instead be used to retrieve the single global instance of the
- * manager.
+ * The ProcessManager is constructed by the CameraManager. This function shall
+ * be used to retrieve the single instance of the manager.
  *
  * \return The Process manager instance
  */
 ProcessManager *ProcessManager::instance()
 {
-	static ProcessManager processManager;
-	return &processManager;
+	return self_;
 }
 
 /**
diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp
index b024f002..2a826222 100644
--- a/test/log/log_process.cpp
+++ b/test/log/log_process.cpp
@@ -132,6 +132,8 @@ private:
 		exitCode_ = exitCode;
 	}
 
+	ProcessManager processManager_;
+
 	Process proc_;
 	Process::ExitStatus exitStatus_;
 	string logPath_;
diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
index 42a26749..a3eaef80 100644
--- a/test/process/process_test.cpp
+++ b/test/process/process_test.cpp
@@ -87,6 +87,8 @@ private:
 		exitCode_ = exitCode;
 	}
 
+	ProcessManager processManager_;
+
 	Process proc_;
 	enum Process::ExitStatus exitStatus_;
 	int exitCode_;