apps: cam: Use signalfd

Use a signalfd to detect `SIGINT` instead of registering a signal handler in
order to remove the `CamApp` singleton. This is better than using a normal
signal handler: a signal can arrive after the `CamApp` object has been destroyed,
leading to a use-after-free. Modifying the `CamApp::app_` in the destructor is
not a good alternative because that would not be async signal safe (unless it
is made atomic).

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

src/apps/cam/main.cpp

@@ -10,8 +10,11 @@
 #include <atomic>
 #include <iomanip>
 #include <iostream>
+#include <pthread.h>
 #include <signal.h>
 #include <string.h>
+#include <sys/signalfd.h>
+#include <unistd.h>
 
 #include <libcamera/libcamera.h>
 #include <libcamera/property_ids.h>
@@ -29,13 +32,10 @@ class CamApp
 public:
 	CamApp();
 
-	static CamApp *instance();
-
 	int init(int argc, char **argv);
 	void cleanup();
 
 	int exec();
-	void quit();
 
 private:
 	void cameraAdded(std::shared_ptr<Camera> cam);
@@ -46,32 +46,45 @@ private:
 
 	static std::string cameraName(const Camera *camera);
 
-	static CamApp *app_;
 	OptionsParser::Options options_;
 
+	UniqueFD signalFd_;
+
 	std::unique_ptr<CameraManager> cm_;
 
 	std::atomic_uint loopUsers_;
 	EventLoop loop_;
 };
 
-CamApp *CamApp::app_ = nullptr;
-
 CamApp::CamApp()
 	: loopUsers_(0)
 {
-	CamApp::app_ = this;
-}
-
-CamApp *CamApp::instance()
-{
-	return CamApp::app_;
 }
 
 int CamApp::init(int argc, char **argv)
 {
+	sigset_t ss;
 	int ret;
 
+	sigemptyset(&ss);
+	sigaddset(&ss, SIGINT);
+
+	ret = -pthread_sigmask(SIG_BLOCK, &ss, nullptr);
+	if (ret < 0)
+		return ret;
+
+	signalFd_.reset(signalfd(-1, &ss, SFD_CLOEXEC | SFD_NONBLOCK));
+	if (!signalFd_.isValid())
+		return -errno;
+
+	loop_.addFdEvent(signalFd_.get(), EventLoop::Read, [this] {
+		signalfd_siginfo si;
+		std::ignore = read(signalFd_.get(), &si, sizeof(si));
+
+		std::cout << "Exiting" << std::endl;
+		loop_.exit();
+	});
+
 	ret = parseOptions(argc, argv);
 	if (ret < 0)
 		return ret;
@@ -103,11 +116,6 @@ int CamApp::exec()
 	return ret;
 }
 
-void CamApp::quit()
-{
-	loop_.exit();
-}
-
 int CamApp::parseOptions(int argc, char *argv[])
 {
 	StreamKeyValueParser streamKeyValue;
@@ -205,7 +213,7 @@ void CamApp::cameraRemoved(std::shared_ptr<Camera> cam)
 void CamApp::captureDone()
 {
 	if (--loopUsers_ == 0)
-		EventLoop::instance()->exit(0);
+		loop_.exit(0);
 }
 
 int CamApp::run()
@@ -345,16 +353,6 @@ std::string CamApp::cameraName(const Camera *camera)
 	return name;
 }
 
-namespace {
-
-void signalHandler([[maybe_unused]] int signal)
-{
-	std::cout << "Exiting" << std::endl;
-	CamApp::instance()->quit();
-}
-
-} /* namespace */
-
 int main(int argc, char **argv)
 {
 	CamApp app;
@@ -364,10 +362,6 @@ int main(int argc, char **argv)
 	if (ret)
 		return ret == -EINTR ? 0 : EXIT_FAILURE;
 
-	struct sigaction sa = {};
-	sa.sa_handler = &signalHandler;
-	sigaction(SIGINT, &sa, nullptr);
-
 	if (app.exec())
 		return EXIT_FAILURE;
 

src/apps/cam/meson.build

@@ -56,6 +56,7 @@ cam  = executable('cam', cam_sources,
                       libjpeg,
                       libsdl2,
                       libtiff,
+                      libthreads,
                       libyaml,
                   ],
                   cpp_args : cam_cpp_args,